Thread: Compiler warning
Hi, I encountered the following compiler warning in 8.4dev. Attached is the patch to fix this problem. Is this worth committing? ------------------ tablecmds.c: In function 'DropErrorMsgWrongType': tablecmds.c:606: warning: format not a string literal and no format arguments ------------------ $ gcc -v Using built-in specs. Target: i486-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu 4.3.3-5ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-4.3/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.3 --program-suffix=-4.3 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-targets=all --with-tune=generic --enable-checking=release --build=i486-linux-gnu --host=i486-linux-gnu --target=i486-linux-gnu Thread model: posix gcc version 4.3.3 (Ubuntu 4.3.3-5ubuntu4) Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Attachment
Fujii Masao wrote: > I encountered the following compiler warning in 8.4dev. > Attached is the patch to fix this problem. Is this worth committing? > > ------------------ > tablecmds.c: In function 'DropErrorMsgWrongType': > tablecmds.c:606: warning: format not a string literal and no format arguments > ------------------ Hmm, it is a false alarm, but would be nice to have a warning-free build. I don't see that warning here on gcc 4.3.3 by default, but I do when I set -Wformat-security. I presume you had that set as well. Applied. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Hmm, it is a false alarm, but would be nice to have a warning-free
> build. I don't see that warning here on gcc 4.3.3 by default, but I do
> when I set -Wformat-security. I presume you had that set as well.
Would it be worth having configure probe for that switch and add it to
CFLAGS if available?
regards, tom lane
On Wednesday 20 May 2009 16:24:21 Tom Lane wrote: > Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes: > > Hmm, it is a false alarm, but would be nice to have a warning-free > > build. I don't see that warning here on gcc 4.3.3 by default, but I do > > when I set -Wformat-security. I presume you had that set as well. > > Would it be worth having configure probe for that switch and add it to > CFLAGS if available? Note that applying this patch would introduce a double-translation issue of the sort that you had complained about a while ago. I'm unsure which way to proceed here.
Peter Eisentraut wrote: > Note that applying this patch would introduce a double-translation issue of > the sort that you had complained about a while ago. I'm unsure which way to > proceed here. Hmm, the patch looks fine to me. The strings are marked with gettext_noop() in the array, and passed through _() when used in errmsg. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Thursday 21 May 2009 10:01:59 Heikki Linnakangas wrote:
> Peter Eisentraut wrote:
> > Note that applying this patch would introduce a double-translation issue
> > of the sort that you had complained about a while ago. I'm unsure which
> > way to proceed here.
>
> Hmm, the patch looks fine to me. The strings are marked with
> gettext_noop() in the array, and passed through _() when used in errmsg.
But his patch changes that to
errhint("%s", _(wentry->drophint_msg))
so it ends up being run through gettext twice. Which has recently been raised
as a concern. Both of these competing issues -- the compiler warning and
double translation -- appear to be minor in practice, but we apparently have
to make a choice which one we plan to fix now and in the future.
Peter Eisentraut <peter_e@gmx.net> writes:
> On Thursday 21 May 2009 10:01:59 Heikki Linnakangas wrote:
>> Hmm, the patch looks fine to me. The strings are marked with
>> gettext_noop() in the array, and passed through _() when used in errmsg.
> But his patch changes that to
> errhint("%s", _(wentry->drophint_msg))
> so it ends up being run through gettext twice.
How so ? errhint only passes its first argument through gettext.
regards, tom lane
On Thursday 21 May 2009 14:29:51 Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On Thursday 21 May 2009 10:01:59 Heikki Linnakangas wrote:
> >> Hmm, the patch looks fine to me. The strings are marked with
> >> gettext_noop() in the array, and passed through _() when used in errmsg.
> >
> > But his patch changes that to
> >
> > errhint("%s", _(wentry->drophint_msg))
> >
> > so it ends up being run through gettext twice.
>
> How so ? errhint only passes its first argument through gettext.
Yeah, not thinking clearly. So I guess this patch is OK if people care about
that warning.