[4/5] kconfig: disable non-literal format string warnings
diff mbox series

Message ID 20190626135546.50665-5-roger.pau@citrix.com
State New, archived
Headers show
Series
  • build improvements/fixes after b41666f2c1
Related show

Commit Message

Roger Pau Monné June 26, 2019, 1:55 p.m. UTC
Kconfig makes heavy use of non-literals as format strings, disable
compiler warnings since this is expected usage.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Doug Goldstein <cardoe@cardoe.com>
---
 xen/tools/kconfig/Makefile.kconfig | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jan Beulich June 26, 2019, 2:45 p.m. UTC | #1
>>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
> Kconfig makes heavy use of non-literals as format strings, disable
> compiler warnings since this is expected usage.

I've never seen any with any version of gcc - are there more
aspects to be mentioned here?

> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Doug Goldstein <cardoe@cardoe.com>
> ---
>  xen/tools/kconfig/Makefile.kconfig | 5 +++++
>  1 file changed, 5 insertions(+)

You Cc list looks to be too short for this change.

> --- a/xen/tools/kconfig/Makefile.kconfig
> +++ b/xen/tools/kconfig/Makefile.kconfig
> @@ -43,6 +43,11 @@ FORCE:
>  # Sets toolchain binaries to use
>  include $(XEN_ROOT)/config/$(shell uname -s).mk
>  
> +# Disable format warnings, kconfig makes heavy use of non-literal format
> +# strings.
> +HOSTCFLAGS += -Wno-format
> +HOSTCXXFLAGS += -Wno-format

But this disables far more warnings than ones for non-literal format
strings, at least afaict.

Jan
Roger Pau Monné June 26, 2019, 3:20 p.m. UTC | #2
On Wed, Jun 26, 2019 at 08:45:27AM -0600, Jan Beulich wrote:
> >>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
> > Kconfig makes heavy use of non-literals as format strings, disable
> > compiler warnings since this is expected usage.
> 
> I've never seen any with any version of gcc - are there more
> aspects to be mentioned here?

Oh, I've always seen them with clang. Not sure why gcc doesn't show
such warnings.

clang -Wp,-MD,tools/kconfig/.conf.o.d    -DCURSES_LOC="<ncurses.h>" -DLOCALE -DKBUILD_NO_NLS  -c -o tools/kconfig/conf.o tools/kconfig/conf.c
tools/kconfig/conf.c:77:10: warning: format string is not a string literal (potentially insecure)
      [-Wformat-security]
                printf(_("aborted!\n\n"));
                       ^~~~~~~~~~~~~~~~~
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^~~~~~~~~~~~~
tools/kconfig/conf.c:77:10: note: treat the string as an argument to avoid this
                printf(_("aborted!\n\n"));
                       ^
                       "%s",
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^
tools/kconfig/conf.c:78:10: warning: format string is not a string literal (potentially insecure)
      [-Wformat-security]
                printf(_("Console input/output is redirected. "));
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^~~~~~~~~~~~~
tools/kconfig/conf.c:78:10: note: treat the string as an argument to avoid this
                printf(_("Console input/output is redirected. "));
                       ^
                       "%s",
tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
#define _(text) gettext(text)
                ^
[...]

> 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Doug Goldstein <cardoe@cardoe.com>
> > ---
> >  xen/tools/kconfig/Makefile.kconfig | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> You Cc list looks to be too short for this change.

That's what get_maintainer.pl has given me. Maybe the syntax in
MAINTAINERS is not correct, or get_maintainer.pl needs adjustments.

> 
> > --- a/xen/tools/kconfig/Makefile.kconfig
> > +++ b/xen/tools/kconfig/Makefile.kconfig
> > @@ -43,6 +43,11 @@ FORCE:
> >  # Sets toolchain binaries to use
> >  include $(XEN_ROOT)/config/$(shell uname -s).mk
> >  
> > +# Disable format warnings, kconfig makes heavy use of non-literal format
> > +# strings.
> > +HOSTCFLAGS += -Wno-format
> > +HOSTCXXFLAGS += -Wno-format
> 
> But this disables far more warnings than ones for non-literal format
> strings, at least afaict.

Sorry, it should be -Wno-format-security. I think I dropped the
-security part while copying.

Thanks, Roger.
Andrew Cooper June 26, 2019, 4:22 p.m. UTC | #3
On 26/06/2019 16:20, Roger Pau Monné wrote:
> On Wed, Jun 26, 2019 at 08:45:27AM -0600, Jan Beulich wrote:
>>>>> On 26.06.19 at 15:55, <roger.pau@citrix.com> wrote:
>>> Kconfig makes heavy use of non-literals as format strings, disable
>>> compiler warnings since this is expected usage.
>> I've never seen any with any version of gcc - are there more
>> aspects to be mentioned here?
> Oh, I've always seen them with clang. Not sure why gcc doesn't show
> such warnings.
>
> clang -Wp,-MD,tools/kconfig/.conf.o.d    -DCURSES_LOC="<ncurses.h>" -DLOCALE -DKBUILD_NO_NLS  -c -o tools/kconfig/conf.o tools/kconfig/conf.c
> tools/kconfig/conf.c:77:10: warning: format string is not a string literal (potentially insecure)
>       [-Wformat-security]
>                 printf(_("aborted!\n\n"));
>                        ^~~~~~~~~~~~~~~~~
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^~~~~~~~~~~~~
> tools/kconfig/conf.c:77:10: note: treat the string as an argument to avoid this
>                 printf(_("aborted!\n\n"));
>                        ^
>                        "%s",
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^
> tools/kconfig/conf.c:78:10: warning: format string is not a string literal (potentially insecure)
>       [-Wformat-security]
>                 printf(_("Console input/output is redirected. "));
>                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^~~~~~~~~~~~~
> tools/kconfig/conf.c:78:10: note: treat the string as an argument to avoid this
>                 printf(_("Console input/output is redirected. "));
>                        ^
>                        "%s",
> tools/kconfig/lkc.h:34:17: note: expanded from macro '_'
> #define _(text) gettext(text)
>                 ^

Clang is correct and GCC is wrong.  This code is plain buggy.

Its trivial to arrange for gettext to return a string with a % in it.

These want fixing to "%s", _(), or to use puts().

It looks like Linux has dropped the use of gettext in the first place. 
Look like c/s 694c49a7c01cc87194be40cb26404b58b68c291c wants backporting.

~Andrew

Patch
diff mbox series

diff --git a/xen/tools/kconfig/Makefile.kconfig b/xen/tools/kconfig/Makefile.kconfig
index 138bf3f1b7..763de37a14 100644
--- a/xen/tools/kconfig/Makefile.kconfig
+++ b/xen/tools/kconfig/Makefile.kconfig
@@ -43,6 +43,11 @@  FORCE:
 # Sets toolchain binaries to use
 include $(XEN_ROOT)/config/$(shell uname -s).mk
 
+# Disable format warnings, kconfig makes heavy use of non-literal format
+# strings.
+HOSTCFLAGS += -Wno-format
+HOSTCXXFLAGS += -Wno-format
+
 # include the original Makefile and Makefile.host from Linux
 include $(src)/Makefile
 include $(src)/Makefile.host