if (smap_opt->answer && sdxy_opt->answer)
- G_fatal_error(_("Both seed map and coordinates cannot be specified"));
+ G_fatal_error("%s", _("Both seed map and coordinates cannot be specified"));
if (smap_opt->answer && sdxy_opt->answer)
- G_fatal_error(_("Both seed map and coordinates cannot be specified"));
+ G_fatal_error("%s", _("Both seed map and coordinates cannot be specified"));
if (smap_opt->answer && sdxy_opt->answer)
- G_fatal_error(_("Both seed map and coordinates cannot be specified"));
+ G_fatal_error("%s", _("Both seed map and coordinates cannot be specified"));
- G_fatal_error(_("Both seed map and coordinates cannot be specified"));
+ G_fatal_error("%s", _("Both seed map and coordinates cannot be specified"));
Martin:
sorry for ignorance, but what is the sense of such changes?
Vaclav:
> the older version triggers a GCC warning complaining
> about format string.
gcc 4.7 or ?
Markus N wrote:
> then it should be changes in *all* GRASS.
better clarify that it is a false positive or real problem before
cluttering up the entire codebase for a dubious and/or needless
change.
short of this being demonstrated as a real problem, my vote would
be to not mess up the entire codebase and tell gcc to be quiet or
get gcc to fix their test.
On Thu, Aug 9, 2012 at 10:35 PM, Hamish <hamish_b@yahoo.com> wrote:
- G_fatal_error(_("Both seed map and coordinates cannot be specified"));
+ G_fatal_error("%s", _("Both seed map and coordinates cannot be specified"));
Martin:
sorry for ignorance, but what is the sense of such changes?
Vaclav:
> the older version triggers a GCC warning complaining
> about format string.
gcc 4.7 or ?
Markus N wrote:
> then it should be changes in *all* GRASS.
better clarify that it is a false positive or real problem before
cluttering up the entire codebase for a dubious and/or needless
change.
short of this being demonstrated as a real problem, my vote would
be to not mess up the entire codebase and tell gcc to be quiet or
get gcc to fix their test.
+1
I don't get any warning about the format string. IMHO much more
interesting are the warnings in r.lake about not initialized file
descriptors.
Any idea why the warning about format strings is triggered for
G_warning() and G_fatal_error(), but not for G_message()? I assume
that G_message would have been affected as well because the relevant
code seems to be identical. Interestingly, in g.proj, G_message()
seems to trigger the same (bogus?) compiler warning, according to the
change log.
Any idea why this affects only r.lake and not other modules, apart
from g.proj, and there also G_message()?
If this is a real problem, why fix the symptoms and not the cause?
That would probably be the macro defining _().
Hello,
just to clarify.
It's not a GCC bug but a valid warning of an exploitable issue: http://en.wikipedia.org/wiki/Uncontrolled_format_string
Still for most of GRASS codebase it's harmless as strings are fixed
and not user provided.
Those, who don't see any warnings, should try CFLAGS="-Wall -Wformat
-Wno-format-extra-args -Wformat-security -Wformat-nonliteral
-Wformat=2" make
The question now is - why we shouldn't change code to get rid of most
of compiler warnings? Gazzillion of irrelevant warnings might just
hide some more important ones thus I personally would be +1 for
working around most common warnings to make compilation more silent
and thus more easy to spot any new issues.
Maris.
PS. Markus M - please provide Your CFLAGS as I don't see any warnings
when compiling r.lake with gcc version 4.6.3 (Gentoo 4.6.3 p1.3,
pie-0.5.1).
2012/8/10 Markus Neteler <neteler@osgeo.org>:
On Fri, Aug 10, 2012 at 12:39 AM, Markus Metz
<markus.metz.giswork@gmail.com> wrote:
...
If this is a real problem, why fix the symptoms and not the cause?
That would probably be the macro defining _().
+1
It worked for the past 10-xx years, so I don't see a reason why
messing up the code when one specific gcc version complains.
On Fri, Aug 10, 2012 at 7:45 AM, Maris Nartiss <maris.gis@gmail.com> wrote:
Hello,
just to clarify.
It's not a GCC bug but a valid warning of an exploitable issue: http://en.wikipedia.org/wiki/Uncontrolled_format_string
Still for most of GRASS codebase it's harmless as strings are fixed
and not user provided.
Those, who don't see any warnings, should try CFLAGS="-Wall -Wformat
-Wno-format-extra-args -Wformat-security -Wformat-nonliteral
-Wformat=2" make
You can also use "pscan" (comes with Debian Fedora, ...):
./lib/gis/datum.c:115 SECURITY: sprintf call should have "%s" as argument 1
./lib/gis/datum.c:120 SECURITY: sprintf call should have "%s" as argument 1
./lib/gis/view.c:465 SECURITY: fprintf call should have "%s" as argument 1
My compile flags are -Wall -Wextra -fno-common -fexceptions
-Werror=implicit-function-declaration
These are not enough to trigger the warnings, the gcc 4.6.3 I use
needs at least one of
-Wall -Wformat -Wno-format-extra-args -Wformat-security
-Wformat-nonliteral -Wformat=2
With regard to r.lake, the warnings seem to be bogus because the
contents of the message strings are out of user control.
If these warnings should be fixed, I guess the right place is either
fixing the _ macro or G_message(), G_warning(), G_fatal_error(). Any
user input always goes first through the parser, thus the parser could
be modified and afterwards the extra format-related -W flags are no
longer needed.
In any case it seems to me that fixing that for every instance of
G_message(), G_warning(), G_fatal_error() and any print functions is
not really feasible.
just to clarify.
It's not a GCC bug but a valid warning of an exploitable issue:
No it isn't. GCC cannot tell the difference between an exploitable
case and a non-exploitable case unless the format string is a literal,
but internationalisation requires dynamic format strings.
http://en.wikipedia.org/wiki/Uncontrolled_format_string
Still for most of GRASS codebase it's harmless as strings are fixed
and not user provided.
Those, who don't see any warnings, should try CFLAGS="-Wall -Wformat
-Wno-format-extra-args -Wformat-security -Wformat-nonliteral
-Wformat=2" make
The question now is - why we shouldn't change code to get rid of most
of compiler warnings?
Because it's more correct to change the compiler options. If you don't
want lots of bogus warnings, don't use warning flags which have a high
proportion of false positives.
Gazzillion of irrelevant warnings might just
hide some more important ones thus I personally would be +1 for
working around most common warnings to make compilation more silent
and thus more easy to spot any new issues.
If you want to change the code to eliminate warnings, start with the
ones which indicate genuine issues with the code, such as
uninitialised or unused variables, type mismatches, etc.
Actually, on second thoughts, don't. That task should be left to
someone who prefers correctness over hiding warnings. There have been
too many *real* bugs hidden by simplistic "fix warning" changes,
mostly a result of hiding type-mismatch warnings by inserting a type
cast, when in fact the data really did have the wrong type.
>> sorry for ignorance, but what is the sense of such changes?
>
> None. Fixed in r52626.
There is another on in r52516.
Not any more
FWIW, if anyone wants to check for genuine format string issues,
configure --without-nls then compile with -Wformat-security. With NLS
disabled, the _() macro just returns its argument, so you won't get
warnings for translated string literals.
