[libcxx] r174642 - Belt and suspenders when calling sysconf
Dave Zarzycki
zarzycki at apple.com
Thu Feb 7 11:10:36 PST 2013
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 <hhinnant at apple.com> 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 <zarzycki at apple.com> 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 <mclow at qualcomm.com> 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
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list