Win32: use _strnicmp (ISO C++) instead of deprecated strnicmp (POSIX) - V2 #1351
Conversation
src/init.cpp
Outdated
There was a problem hiding this comment.
Rather move this to an #else under the #ifndef WIN32...
|
Updated to reflect suggestion from @luke-jr. |
|
NACK. It would be better to get rid of usage of strncasecmp all together and use boost's case-insensitive string support. E.g. istarts_with is in boost/algorithm/string/predicate.hpp |
|
@gavinandresen Are you fine with massive boost usage now? I will update that pull and use what you suggested, no problem. |
|
Closed in favor of #1363! |
|
RE: "massive boost usage" : I'm all for making the code clearer and more cross-platform, and we're dependent on boost anyway. The string algorithms are nice little self-contained pieces of functionality. But I'm on the fence for changes like sipa's pull request that uses boost::variant for bitcoin addresses (and requires writing little visitor classes to do simple things). I think template metaprogramming is the wrong way to go, it makes code hard to debug, creates cryptic compiler messages, etc. And I was using boost's compile-time regex library for a pull request, but one of the reasons it never got pulled was because it would increase compile times and memory usage when compiling. |
|
creates cryptic compiler messages, etc. One nitpick: you could use clang. In contrary to gcc, it has wonderfully clear error messages, even with template metaprogramming. Also it compiles much faster. But I agree that usage of boost should improve readability, not decrease it. And compile-time work is great for time-sensitive inner loops that need to be optimized for specific cases, but using it unnecessarily increases code size and compile time. |
- allow filtering by all signals in `list` - do not count/print funding votes for watchdogs
[fix for miner_tests.cpp] Initialize nThreads in parallel.cpp
3f98b37 [CMake] Check for and require Qt 5.5.1 minimum version (Fuzzbawls) 19ce9de build: Refactor out Qt version check macro (Fuzzbawls) 2c18e82 Bump minimum required Qt to 5.5.1 (Fuzzbawls) Pull request description: As per bitcoin#1292, this effectively bumps the minimum required Qt version to 5.5.1. - Removed no longer needed precompiler conditionals - Updated autotools to check for and require at least 5.5.1 to build the GUI - Updated CMake to check for and require at least 5.5.1 to build the `pivx-qt` target Closes bitcoin#1292 ACKs for top commit: random-zebra: ACK 3f98b37 furszy: looking good, utACK 3f98b37 Tree-SHA512: 32e494ea7f166ce57a9be08dbfcc067546f4e0c46ccc8b9a4836dd148243ca14e1fede166cccdb6ae6ddbae8f0832362a6935e42742ed957b41be3104511bdd0
9450439 build: remove extra tokens warning (Fuzzbawls) ebb4da9 build: fix m4 escaping (Fuzzbawls) 2b3b9c3 doc: proper capitalize terms (Fuzzbawls) efee3fc build: remove duplicated QT_STATICPLUGIN define (Fuzzbawls) Pull request description: Further cleanups to the Qt m4 file that I wanted to keep separate from bitcoin#1351 ACKs for top commit: random-zebra: ACK 9450439 furszy: ACK 9450439 Tree-SHA512: 1d8c0194eb72116ea4ee414c31cec8a6f472c3493a0aa63e456188aff3a2c20b8871fba22b27947ebd303b0ebe42b86a2cbaed42d3ee424a69088d8e61070b34
c54fd9d [Qt] Switch to newer connect syntax (Fuzzbawls) Pull request description: Switch all Qt connections to the newer functor-based syntax. Marked [WIP] for now until larger PRs can be merged. Mostly ported from bitcoin#13529 Requires the following PR to be merged first in order to adhere to standards: - [x] bitcoin#1351 ACKs for top commit: furszy: ACK c54fd9d random-zebra: ACK c54fd9d and merging... Tree-SHA512: d5222264566a8920ab97e8c586f134156d9d0514d014b430bdd47a34b0620ab94a1c6d341b6a57322f1131a8fad91a5e1af907f3bd7112d0ab5e04c4cc9f5ae5
4 participants
This does things better than the first commit, which had a bug and placed
define strncasecmp _strnicmpin an#ifndef WIN32in init.cpp. Sorry for this.