To be clear, I am *not* objecting to this patch. It is clearly safe in this specific case.
My point is that replacing "== -1" with "< 0" is a common POSIX anti-pattern. If negative values do not make sense, then arguably an assert() is appropriate if one wants to be paranoid. davez On Feb 7, 2013, at 2:04 PM, Howard Hinnant <[email protected]> wrote: > Fwiw, Marshall discussed this change with me before committing. That being > said, I have no objection to reverting it either. And either way we go, this > code does not execute on Apple platforms (#ifdef'd out). > > Howard > > On Feb 7, 2013, at 1:54 PM, Dave Zarzycki <[email protected]> wrote: > >> Marshall, >> >> FYI — While safe in this specific example, this is not a correct transform >> to make in the general case. POSIX very clearly specifies that -1 is the >> error code for this and similar APIs. This matters for APIs like mmap() >> which can return "negative" values that not an error. >> >> davez >> >> >> On Feb 7, 2013, at 1:48 PM, Marshall Clow <[email protected]> wrote: >> >>> Author: marshall >>> Date: Thu Feb 7 12:48:09 2013 >>> New Revision: 174642 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=174642&view=rev >>> Log: >>> Belt and suspenders when calling sysconf >>> >>> Modified: >>> libcxx/trunk/src/thread.cpp >>> >>> Modified: libcxx/trunk/src/thread.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/thread.cpp?rev=174642&r1=174641&r2=174642&view=diff >>> ============================================================================== >>> --- libcxx/trunk/src/thread.cpp (original) >>> +++ libcxx/trunk/src/thread.cpp Thu Feb 7 12:48:09 2013 >>> @@ -69,7 +69,9 @@ thread::hardware_concurrency() _NOEXCEPT >>> long result = sysconf(_SC_NPROCESSORS_ONLN); >>> // sysconf returns -1 if the name is invalid, the option does not exist or >>> // does not have a definite limit. >>> - if (result == -1) >>> + // if sysconf returns some other negative number, we have no idea >>> + // what is going on. Default to something safe. >>> + if (result < 0) >>> return 0; >>> return static_cast<unsigned>(result); >>> #else // defined(CTL_HW) && defined(HW_NCPU) >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
