On Mon, Dec 16, 2013 at 12:19 PM, Thomas H.P. Andersen <pho...@gmail.com> wrote: > On Mon, Dec 16, 2013 at 8:15 PM, Shawn Landden <sh...@churchofgit.com> wrote: >> On Mon, Dec 16, 2013 at 11:06 AM, Thomas H.P. Andersen <pho...@gmail.com> >> wrote: >>> On Mon, Dec 16, 2013 at 7:50 PM, Shawn Landden <sh...@churchofgit.com> >>> wrote: >>>> On Mon, Dec 16, 2013 at 10:36 AM, Lennart Poettering >>>> <lenn...@poettering.net> wrote: >>>>> On Mon, 16.12.13 09:20, Shawn Landden (sh...@churchofgit.com) wrote: >>>>> >>>>>> While all the libc implementations I know return NULL when memchr's size >>>>>> parameter is 0: >>>>>> >>>>>> C11 7.24.1p2: Where an argument declared as "size_t n" specifies the >>>>>> length >>>>>> of the array for a function, n can have the value zero on a call to that >>>>>> function. Unless explicitly stated otherwise in the description of a >>>>>> particular function in this subclause, pointer arguments on such a call >>>>>> shall still have valid values, as described in 7.1.4. On such a call, a >>>>>> function that locates a character finds no occurrence, a function that >>>>>> compares two character sequences returns zero, and a function that copies >>>>>> characters copies zero characters. >>>>>> >>>>>> see http://llvm.org/bugs/show_bug.cgi?id=18247 >>>>> >>>>> Hmm? But what does that have to do with the requirements we make on our >>>>> own internal functions? >>>> Well, it makes clang's scan-build shut up :), which is how i initially >>>> came across it. >>> >>> Unfortunately there are quite a lot of false positives in scan-build. >>> I have been cleaning it up for a while. I did come across this one >>> earlier. Isn't the issue here that scan-build makes the wrong >>> assumption that data can be null at that point? Generally it gets >>> confused sometimes about functions that return a non-negative value >>> only when setting some data succeeded. At least I think that was the >>> conclusion I came to about the report. >> While there are certainly those (thinking walking a linked-list is >> use-after free, >> _cleanup_ issues: http://llvm.org/bugs/show_bug.cgi?id=3888 ) >> this was a real bug that scan_build found, but which I don't totally agree >> is >> important. If cause that there wasn't assert() against the first argument to >> memchr() being NULL, instead there was an assert against the first argument >> not being NULL OR size being 0, which (I was corrected to accept) can >> invoke undefined behavior. > After rereading it I see that I was mistaken. I had thought that data > would already be set at that point if size == 0. Sorry for the > confusion. > > Would it be enough to just add: > if (data == NULL && size == 0) || > eq = false; NULL > else > eq = memchr(data, '=', size); or keep the existing assert() and add if (size == 0) eq = NULL; else eq = memchr(data, '=', size);
> _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
