On 2 November 2016 at 23:07, Jason Merrill <ja...@redhat.com> wrote: > On Wed, Nov 2, 2016 at 1:08 PM, Prathamesh Kulkarni > <prathamesh.kulka...@linaro.org> wrote: >> On 2 November 2016 at 18:29, Jason Merrill <ja...@redhat.com> wrote: >>> Then I'll approve the whole patch. >> Thanks! >> Trying the patch on kernel build (allmodconfig) reveals the following >> couple of warnings: >> http://pastebin.com/Sv2HFDUv >> >> I think warning for str_error_r() is correct > > It's accurate, but unhelpful; snprintf isn't going to use the contents > of buf via the variadic argument, so this warning is just noise. Ah, indeed, it's just printing address of buf, not using the contents. > >> however I am not sure if >> warning for pager_preexec() is legit or a false positive: >> >> pager.c: In function 'pager_preexec': >> pager.c:35:12: warning: passing argument 2 to restrict-qualified >> parameter aliases with argument 4 [-Wrestrict] >> select(1, &in, NULL, &in, NULL); >> ^~~ ~~~ >> Is the warning correct for the above call to select() syscall ? > > The warning looks correct based on the prototype > > extern int select (int __nfds, fd_set *__restrict __readfds, > fd_set *__restrict __writefds, > fd_set *__restrict __exceptfds, > struct timeval *__restrict __timeout); > > But passing the same fd_set to both readfds and exceptfds seems > reasonable to me, so this also seems like a false positive. > > Looking at C11, I see this example: > > EXAMPLE 3 The function parameter declarations > void h(int n, int * restrict p, int * restrict q, int * restrict r) > { > int i; > for (i = 0; i < n; i++) > p[i] = q[i] + r[i]; > } > > illustrate how an unmodified object can be aliased through two > restricted pointers. In particular, if a and b > are disjoint arrays, a call of the form h(100, a, b, b) has defined > behavior, because array b is not > modified within function h. > > This is is another example of well-defined code that your warning will > complain about. Yes, that's a limitation of the patch, it just looks at the prototype, and not how the arguments are used in the function. > >> Should we instead keep it in Wextra, or continue keeping it in Wall ? > > It seems that it doesn't belong in -Wall. I don't feel strongly about > -Wextra. Should I commit the patch by keeping Wrestrict "standalone", ie, not including it in either Wall or Wextra ?
Thanks, Prathamesh > > Jason