Re: [PATCH] src/sort.c: assert on temp.text before calling memcpy()
Hey, On Tue January 5 2010 21:01:13 Kovarththanan Rajaratnam wrote: > clang detected the following false positive: > > sort.c:2292:7: warning: Null pointer passed as an argument to a 'nonnull' > parameter memcpy (temp.text, line->text, line->length); how can I force clang to issue the warning? Is it necessary to have a recent version and/or use any extra options? Thanks in advance for your answer! Kamil
Re: [PATCH] src/sort.c: assert on temp.text before calling memcpy()
Pádraig Brady wrote: > That looks wrong. > Should memcpy() be marked as nonnull as the length can be 0? That doesn't matter: C99 7.21.1#2 "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." > In any case we don't want to be asserting in such cases, > rather just doing: > > if (temp.text) > memcpy (...) >From what I could gather all paths seem to lead to a temp.text which is non null. That is why I preferred the assert(). Is this assumption incorrect? -- Best regards, Kovarththanan Rajaratnam
Re: [PATCH] src/sort.c: assert on temp.text before calling memcpy()
On 06/01/10 11:16, Kovarththanan Rajaratnam wrote: Pádraig Brady wrote: That looks wrong. Should memcpy() be marked as nonnull as the length can be 0? That doesn't matter: C99 7.21.1#2 "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." Seems like a silly memcpy implementation that would touch the pointers when n==0 In any case we don't want to be asserting in such cases, rather just doing: if (temp.text) memcpy (...) From what I could gather all paths seem to lead to a temp.text which is non null. That is why I preferred the assert(). Is this assumption incorrect? Having checked the code, I don't think it's incorrect as line->length is >=1, though why couldn't clang figure that out? Will we have to be putting asserts before loads of these nonnull functions now? cheers, Pádraig
Re: [PATCH] src/sort.c: assert on temp.text before calling memcpy()
That looks wrong. Should memcpy() be marked as nonnull as the length can be 0? In any case we don't want to be asserting in such cases, rather just doing: if (temp.text) memcpy (...)
[PATCH] src/sort.c: assert on temp.text before calling memcpy()
>From c65f5323d2c187e01e73c7a074382f77ff8ee708 Mon Sep 17 00:00:00 2001 From: Kovarththanan Rajaratnam Date: Tue, 5 Jan 2010 20:24:02 +0100 Subject: [PATCH] src/sort.c: assert on temp.text before calling memcpy() clang detected the following false positive: sort.c:2292:7: warning: Null pointer passed as an argument to a 'nonnull' parameter memcpy (temp.text, line->text, line->length); When we reach this line temp.text is guaranteed to be non null. --- src/sort.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/sort.c b/src/sort.c index 481fdb8..b50f6cd 100644 --- a/src/sort.c +++ b/src/sort.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "system.h" #include "argmatch.h" #include "error.h" @@ -2289,6 +2290,7 @@ check (char const *file_name, char checkonly) temp.text = xrealloc (temp.text, alloc); } + assert(temp.text); memcpy (temp.text, line->text, line->length); temp.length = line->length; if (key) -- 1.6.3.3
