[ I haven't reviewed the original patch ] On Wed, Oct 09, 2019 at 03:26:18PM +0200, Rasmus Villemoes wrote: > On 09/10/2019 14.14, Markus Elfring wrote: > > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > > Date: Wed, 9 Oct 2019 13:53:59 +0200 > > > > Several functions return values with which useful data processing > > should be performed. These values must not be ignored then. > > Thus use the annotation “__must_check” in the shown function declarations. > > This _might_ make sense for those that are basically kmalloc() wrappers > in one way or another [1]. But what's the point of annotating pure > functions such as strchr, strstr, memchr etc? Nobody is calling those > for their side effects (they don't have any...), so obviously the return > value is used. If somebody does a strcmp() without using the result, so > what? OK, it's odd code that might be worth flagging, but I don't think > that's the kind of thing one accidentally adds. if (ret) { -EINVAL; } People do occasionally make mistakes like this. It can't hurt to warn them as early as possible about nonsense code. > You're also not consistent - strlen() is not annotated. And, for the > standard C functions, -Wall already seems to warn about an unused > call: > > #include <string.h> > int f(const char *s) > { > strlen(s); > return 3; > } > $ gcc -Wall -o a.o -c a.c > a.c: In function ‘f’: > a.c:5:2: warning: statement with no effect [-Wunused-value] > strlen(s); > ^~~~~~~~~ That's because glibc strlen is annotated with __attribute_pure__ which means it has no side effects. regards, dan carpenter