[systemd-devel] [PATCH] Add some extra __attribute__ ((format)) s

[Lennart Poettering]

On Fri, 05.04.13 05:30, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:

> unfortunately this one gives a warning during compilation (gcc-4.7.2-8.fc18.x86_64):
> 
> ../src/core/unit.c: In function 'unit_status_printf':
> ../src/core/unit.c:2556:9: warning: format not a string literal,
> argument types not checked [-Wformat-nonliteral]

Sounds like something to deal with via:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
...
#pragma GCC diagnostic pop

We use it in a couple of other cases too where it is actually OK to pass
a variable as format string...

> > --- a/src/systemd/sd-bus.h
> > +++ b/src/systemd/sd-bus.h
> > @@ -161,7 +161,7 @@ int sd_bus_get_owner_pid(sd_bus *bus, const char *name, pid_t *pid);
> >  #define SD_BUS_ERROR_INIT_CONST(name, message) {(name), (message), 0}
> >  
> >  void sd_bus_error_free(sd_bus_error *e);
> > -int sd_bus_error_set(sd_bus_error *e, const char *name, const char *format, ...);
> > +int sd_bus_error_set(sd_bus_error *e, const char *name, const char *format, ...)  __attribute__ ((format (printf, 3, 0)));
> 
> I don't think that printf attributes should be added unconditionally
> to public functions (this applies to src/system/*.h and src/udev/udev.h).
> The problem is that sometimes gcc emits warnings which are hard to get 
> rid of. src/systemd/sd-daemon.h does a dance with '#define _sd_printf_attr_',
> which at least allows the user of the header to undefine the macro,
> avoiding problems. If we add macro to public headers, this pattern
> should be followed.

Yes, agreed. For public headers in src/systemd/*.h we should do what
sd-daemon.h does. And I think every header should redefine this, if it
needs to, because I'd like to keep the interlinking of the header files
at a minimum.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.