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
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
