-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 AMDG shared/date_time.cpp: line 79: If it's an invariant that tz_ is the same as the timezone of impl, then this is not exception safe if clone throws. You'll have to use copy and swap idiom. line 161: This is a bit odd, don't you think? Normally if assignment is strongly exception safe without doing any extra work, there's no reason to go out of your way to clear the old state on failure. line 350: should you check that v is in range? Regardless, the valid range should be documented. shared/format.cpp: line 53: it seems that this doesn't restore the float field, base field etc. shared/formatting.cpp: line 42: normally copy and swap is the preferred way to implement assignment. The main advantage is that it guarantees that if the operation fails, the state of the lhs is unchanged. shared/generator.hpp: line 41: From the way domains is used, would a std::set be more appropriate? shared/ids.cpp: missing copyright/license. (Issues like this will be caught by the inspect tool, so not a big deal now.) shared/ios_prop.hpp: line 30 and others,: casting to and from void* should use static_cast. line 56: can you use invalid instead of hard-coding this constant? line 96: I don't think this is guaranteed to be distinct from any valid pointer. Can you make invalid the address of a unique global object instead? shared/localization_backend.cpp: lines 200, 206: This is not guaranteed to be thread-safe in C++03. You must use call_once for thread-safe static inititalization. Note however that this is probably safe if you assume that no additional threads are started before main. shared/message.cpp: line 123: I take it that the format guarantees that hash_size_ > 2? Please validate this requirement when loading. line 132: Use null_pair? line 152: You require that off is in range, that off <= off + len, and that off + len is in range. Please check this. line 204: You really should check that you actually read 4 bytes. Otherwise if the file is too short, you get undefined behavior from reading uninitialized memory. line 311: How do you know that there's a null-terminator? line 326: suppose that form is 0 and the null-terminator is missing... line 407: This isn't guaranteed to work. The standard doesn't guarantee that the letters are contiguous. (Does anything use EBCDIC these days?) lines 440, 442: You're assuming that the string is null terminated. What if the file is incorrectly formatted? It may be possible to read past the end of the allocated block. I haven't actually worked through the exact code path to trigger this condition, but it seems suspicious. line 475: You're assuming that the separator actually appears. line 500: I think this can cause alignment problems. Does the file format make any alignment guarantees? Even if it does, you'll need to handle invalid files gracefully. Of course, it'll only be inefficient on platforms that allow unaligned loads, but still, the standard doesn't guarantee that it'll work. shared/mo_hash.hpp: shared/mo_lambda.cpp: line 239: There's no 'c' in tokenizer. Everything except compile can be in the unnamed namespace. shared/mo_lambda.hpp: std/all_generater.hpp: std/codecvt.cpp: std/collate.cpp: std/converter.cpp: std/numeric.cpp: std/std_backend.cpp: std/std_backend.hpp: util/codecvt_converter.cpp: util/default_locale.cpp: util/iconv.hpp: Is this header intended to be used in multiple translation units? If so you probably should make the functions inline and not use an unnamed namespace. util/info.hpp: util/locale_data.hpp: util/locale_data.cpp: util/numeric.cpp: util/timezone.hpp: win32/all_generator.hpp: win32/api.hpp: line 49: Don't most Windows functions use DWORD, rather than int? win32/collate.hpp: line 48: I think I just saw code looking just like this in another file. win32/converter.cpp: win32/lcid.cpp: line 51: Check whether this succeeds and uses the entire string? line 89: I thought double checked locking was considered unsafe? Anyway you should use call_once for initialization like this. win32/lcid.hpp: win32/numeric.cpp: line 32: I think I saw another copy of these functions. Get rid of the duplication? win32/win_backend.cpp: win32/win_backend.hpp: build/Jamfile.v2: line 30: I don't think you need a separate object file. It isn't used anywhere else. line 126: A simpler way to reference another library is /boost//thread line 211: I would tend to use icu-sources = [ glob icu/*.cpp ] ; result += <source>$(icu-sources) I see you're using something like this later on. line 278: Boost.Thread ought to set this flag automatically. I'm assuming from the fact that you have it, that it doesn't. Looks like a bug in the thread Jamfile. (Similarly, you should add <define>BOOST_LOCALE_NO_LIB in the usage requirements of boost_locale) test/Jamfile.v2: You don't actually need test-suite. test-suite is exactly the same as alias. In Christ, Steven Watanabe -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNp7p5AAoJEDTBQuhymLHSiv4H/0HCgXhXiOAsMmMFY6BYn9W7 aSKX3n9qGCDTG7WI+tHoZwz46e6BHrf7PlfcA+6Z1v4wA1GBonxxXrmTN8AhNIIm ozn7Qlw7xH13rO7r3BfPPmib0sSOWCTEUcIfCI2ZW4huKuGMOnmI+tLaHr6qwVP3 +xAiWjAzNkShzn8Qy2rWYo9X+mYaXBrBjViejlyn7M0ArW2Rgcsu8FPWh8Kkhzqk 6TcltpHmUI4dQ9uAkJhP9zcjqyKIUA9vTApcKCDJMqGnAwSDoXA+ZWZGuoquhsia 4BAkL32+E9EnpEf982R+3cQ7NqYQy452rB9TC02qsFrYbrPv6FKDmQMb0uWvBGM= =bzjH -----END PGP SIGNATURE-----