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
Attachments:
fix_compiler_warning.patchtext/x-patch; charset=US-ASCII; name=fix_compiler_warning.patchDownload
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.284
diff -r1.284 tablecmds.c
609c609
< (wentry->kind != '\0') ? errhint(wentry->drophint_msg) : 0));
---
> (wentry->kind != '\0') ? errhint("%s", _(wentry->drophint_msg)) : 0));
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.