[cmake-developers] Autogen subdirectories patches

[Ben Boeckel]

On Tue, Jul 26, 2016 at 17:31:00 +0200, Sebastian Holtermann wrote:
> I have prepared an other approach which uses checksum suffixes
> instead of nested directory generation.
> This was tested on my Debian/amd64/Makefile setup.

Interesting; I like it. Might be worthwhile to reuse it for the .o files
in the future as well (since these can conflict, but are much less
likely to do so).

> Please review.

Could you run clang-format over the code? There are a number of style
issues that it will fix (IIRC, it needs at least Clang 3.8; I'm unsure
what version of the clang that comes with Xcode is sufficient).

> +    rccOutputPath += cmQtAutoGeneratorUtil::BuildFileBase( qrcSourceName,
> +      makefile->GetCurrentSourceDirectory(),
> +      makefile->GetCurrentBinaryDirectory(),
> +      makefile->GetHomeDirectory(),
> +      makefile->GetHomeOutputDirectory() );

Is there a reason we can't pass just the `makefile` down and have it
query all the parameters rather than having 5 arguments?

> +      struct TPair {
> +        const std::string * dir;
> +        const char * seed;
> +      };
> +      TPair pDirs[4] = {
> +        { &parenDirCSource, "CurrentSource" },
> +        { &parenDirCBinary, "CurrentBinary" },
> +        { &parenDirPSource, "ProjectSource" },
> +        { &parenDirPBinary, "ProjectBinary" }
> +      };
> +      TPair * itc = &pDirs[0];
> +      TPair * ite = &pDirs[0] + ( sizeof ( pDirs ) / sizeof ( TPair ) );

It might be better to just use a NULL sentinel at the end of the array.
If not, the extra spaces here should go away.

> +  // Calculate the file ( seed + relative path + name ) checksum
> +  std::string checksumBase64;
> +  {
> +    ::std::vector<unsigned char>hashBytes;

CMake just uses std::, not ::std::.

> +    {
> +      // Acquire hex value hash string
> +      std::string hexHash = cmCryptoHash::New("SHA256")->HashString(
> +            (sourceRelSeed+sourceRelPath+sourceFileName).c_str());
> +      // Convert hex value string to bytes
> +      hashBytes.resize ( hexHash.size()/2 );
> +      for ( unsigned int ii=0; ii != hashBytes.size(); ++ii ) {
> +        unsigned char byte ( 0 );
> +        for ( unsigned int jj=0; jj != 2; ++jj ) {
> +          unsigned char cval ( 0 );
> +          switch ( hexHash[ii*2+jj] ) {
> +          case '0': cval=0; break; case '1': cval=1; break;
> +          case '2': cval=2; break; case '3': cval=3; break;
> +          case '4': cval=4; break; case '5': cval=5; break;
> +          case '6': cval=6; break; case '7': cval=7; break;
> +          case '8': cval=8; break; case '9': cval=9; break;
> +          case 'a': cval=10; break; case 'b': cval=11; break;
> +          case 'c': cval=12; break; case 'd': cval=13; break;
> +          case 'e': cval=14; break; case 'f': cval=15; break;
> +          default: break;

I feel like this is better as:

    int nibble = hexHash[ii * 2 + jj];
    if ('0' <= nibble && nibble <= '9') {
        cval = nibble - '0';
    } else if ('a' <= nibble && nibble <= 'f') {
        cval = nibble - 'a' + 10;
    } else {
        // internal error about an unexpected hexchar
    }

Alternatively, cmUuid::IntFromHexDigit() (and possibly other methods)
could be refactored to allow this to share an implementation.

--Ben