On 2012-05-19 at 16:26 +0200, Andreas Metzler wrote: > On 2012-05-18 Andreas Metzler <eximusers@???> wrote:
> > On 2012-05-18 Phil Pennock <pdp@???> wrote:
> > > I have uploaded Exim 4.80 RC1 to:
> [...]
>
> > I get a strange error when building with -Werror=format-security:
Right, because we're using a format check for something which isn't
quite what the check's creators expected.
The __attribute__((format(printf,A,B))) stuff is generally more useful
than not, but Exim is not actually using libc printf, but its own
string_printf(). One of the differences is that string_printf() treats
%D as special, not consuming any arguments.
If you're going to build with -Werror=format-security then you need to
#define PRINTF_FUNCTION(A,B) to /**/ in mytypes.h, which will also shut
up a bunch of other warnings. The PRINTF_FUNCTION() usage has caught a
number of small issues and been generally useful, but it's not a perfect
match. If there were a pragma to define a new format and register what
each escape expects, as a type, and declare that some do not consume
arguments, we could use that and there would be no mismatches.
> > which corresponds to
> > /* Do *not* use "%s" here, we need the %D datestamp in the log_file to
> > be expanded! */
> > (void)string_format(log_file_open, sizeof(log_file_open), CS log_file);
> [...]
>
> Hello,
>
> Reverting a part of e0df1c8324f0e0c4112302fa473cff6a6110a044 makes the
> problem unreproducible:
Right, SuSE folks supplied a patch because we weren't using
PRINTF_FUNCTION() consistently, as I'd originally only applied it to
those places where it did not trigger spurious warnings.
> What also makes exim compile is reverting this part of
> c6e95d22d77f480804ddb5c505891206b427dfb1 (which was a partial revert
> of abovementioned e0df1c8324f0e0c4112302fa473cff6a6110a044):
If you do this, then you break cases of LOG_FILE_PATH containing %D.
I think, realistically, people are going to turn on -Wformat=security
and we need to accept that and remove the safety-checks instead.
They're useful to the developers, in figuring out where there *might* be
issues, but there's so many false positive warnings, and this, that it
is not tenable for a release.
For now, can you please build without -Wformat=security and see if it
works?
