On Mon, Nov 16, 2015 at 8:41 AM, Michael McConville <[email protected]> wrote:
> Craig Rodrigues wrote:
>> Recently, I imported imsg.c from OpenBSD to the
>> FreeBSD base system's libopenbsd:
>>
>> https://svnweb.freebsd.org/changeset/base/290375
>>
>> When compiling on FreeBSD, we get a compiler warning with clang:
>>
>> cc -O2 -pipe -I/opt2/branches/head2/lib/libopenbsd -std=gnu99
>> -fstack-protector-strong -Wsystem-headers -Wall -Wno-format-y2k -W
>> -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes
>> -Wpointer-arith -Wno-uninitialized -Wno-pointer-sign -Wno-empty-body
>> -Wno-string-plus-int -Wno-unused-const-variable -Wno-tautological-compare
>> -Wno-unused-value -Wno-parentheses-equality -Wno-unused-function
>> -Wno-enum-conversion -Wno-unused-local-typedef -Qunused-arguments -c
>> /opt2/branches/head2/lib/libopenbsd/imsg.c -o imsg.o
>> /opt2/branches/head2/lib/libopenbsd/imsg.c:78:6: warning: comparison of
>> integers of different signs: 'unsigned long' and 'int' [-Wsign-compare]
>> >= getdtablesize()) {
>> ^ ~~~~~~~~~~~~~~~
>> 1 warning generated.
Out of curiousity, in your experience how often is that warning a real
issue versus a false positive? In this case it's a false positive and
the code is safe.
>> I can certainly patch the code with a cast on FreeBSD to get rid of
>> the compiler warning. However, I was wondering if there is a good fix
>> that we can share between OpenBSD and FreeBSD?
>
> Both OSs define getdtablesize() as sysconf(_SC_OPEN_MAX). sysconf(3)
> returns a long, so it seems more correct to make getdtablesize(3) return
> a long or ssize_t. The return value has to be signed because sysconf(3)
> is specified by POSIX to return -1 and set errno on failure.
No, fds are ints so returning a long from getdtablesize(3) would be
misleading and useless.
In the full expression:
if (getdtablecount() + imsg_fd_overhead +
(CMSG_SPACE(sizeof(int))-CMSG_SPACE(0))/sizeof(int)
>= getdtablesize()) {
the type of unsigned long just be from the CMSG_SPACE() term; if
keeping that warning option is felt to be a net positive, I would
suggest casting to int just the CMSG-CMSG/sizeof term.
Philip Guenther
