| Message ID | 20230804082619.61833-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | lib/vsprintf: Rework header inclusions | expand |
On Fri, 4 Aug 2023 at 10:26, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > Sparse is not happy to see non-static variable without declaration: > lib/vsprintf.c:61:6: warning: symbol 'no_hash_pointers' was not declared. Should it be static? > > Declare respective variable in the local header. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > lib/test_printf.c | 4 ++-- > lib/vsprintf.c | 1 + > lib/vsprintf.h | 7 +++++++ > mm/kfence/report.c | 3 +-- > 4 files changed, 11 insertions(+), 4 deletions(-) > create mode 100644 lib/vsprintf.h > > diff --git a/lib/test_printf.c b/lib/test_printf.c > index 7677ebccf3c3..9e04b5f7244a 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -24,6 +24,8 @@ > > #include <linux/property.h> > > +#include "vsprintf.h" > + > #include "../tools/testing/selftests/kselftest_module.h" > > #define BUF_SIZE 256 > @@ -41,8 +43,6 @@ KSTM_MODULE_GLOBALS(); > static char *test_buffer __initdata; > static char *alloced_buffer __initdata; > > -extern bool no_hash_pointers; > - > static int __printf(4, 0) __init > do_test(int bufsize, const char *expect, int elen, > const char *fmt, va_list ap) > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index 40f560959b16..6774cf84e623 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -54,6 +54,7 @@ > > #include <linux/string_helpers.h> > #include "kstrtox.h" > +#include "vsprintf.h" > > /* Disable pointer hashing if requested */ > bool no_hash_pointers __ro_after_init; > diff --git a/lib/vsprintf.h b/lib/vsprintf.h > new file mode 100644 > index 000000000000..ddffde905824 > --- /dev/null > +++ b/lib/vsprintf.h > @@ -0,0 +1,7 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _LIB_VSPRINTF_H > +#define _LIB_VSPRINTF_H > + > +extern bool no_hash_pointers; > + > +#endif It seems odd to create such a local header and then refer to it from other subsystems. What's the downside of just putting this into the new include/linux/sprintf.h? If someone wants to access this variable, they will one way or another.
On Fri, 4 Aug 2023 11:26:16 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > diff --git a/mm/kfence/report.c b/mm/kfence/report.c > index 197430a5be4a..fb28c6abd58e 100644 > --- a/mm/kfence/report.c > +++ b/mm/kfence/report.c > @@ -19,6 +19,7 @@ > > #include <asm/kfence.h> > > +#include "../../lib/vsprintf.h" > #include "kfence.h" > Ug, I hate "../../" includes. Can we have this in one of the main header files? -- Steve
On Fri, 4 Aug 2023 10:38:22 +0200 Marco Elver <elver@google.com> wrote: > > +++ b/lib/vsprintf.h > > @@ -0,0 +1,7 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LIB_VSPRINTF_H > > +#define _LIB_VSPRINTF_H > > + > > +extern bool no_hash_pointers; > > + > > +#endif > > It seems odd to create such a local header and then refer to it from > other subsystems. > > What's the downside of just putting this into the new > include/linux/sprintf.h? If someone wants to access this variable, > they will one way or another. +1 -- Steve
diff --git a/lib/test_printf.c b/lib/test_printf.c index 7677ebccf3c3..9e04b5f7244a 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -24,6 +24,8 @@ #include <linux/property.h> +#include "vsprintf.h" + #include "../tools/testing/selftests/kselftest_module.h" #define BUF_SIZE 256 @@ -41,8 +43,6 @@ KSTM_MODULE_GLOBALS(); static char *test_buffer __initdata; static char *alloced_buffer __initdata; -extern bool no_hash_pointers; - static int __printf(4, 0) __init do_test(int bufsize, const char *expect, int elen, const char *fmt, va_list ap) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 40f560959b16..6774cf84e623 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -54,6 +54,7 @@ #include <linux/string_helpers.h> #include "kstrtox.h" +#include "vsprintf.h" /* Disable pointer hashing if requested */ bool no_hash_pointers __ro_after_init; diff --git a/lib/vsprintf.h b/lib/vsprintf.h new file mode 100644 index 000000000000..ddffde905824 --- /dev/null +++ b/lib/vsprintf.h @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LIB_VSPRINTF_H +#define _LIB_VSPRINTF_H + +extern bool no_hash_pointers; + +#endif diff --git a/mm/kfence/report.c b/mm/kfence/report.c index 197430a5be4a..fb28c6abd58e 100644 --- a/mm/kfence/report.c +++ b/mm/kfence/report.c @@ -19,6 +19,7 @@ #include <asm/kfence.h> +#include "../../lib/vsprintf.h" #include "kfence.h" /* May be overridden by <asm/kfence.h>. */ @@ -26,8 +27,6 @@ #define ARCH_FUNC_PREFIX "" #endif -extern bool no_hash_pointers; - /* Helper function to either print to a seq_file or to console. */ __printf(2, 3) static void seq_con_printf(struct seq_file *seq, const char *fmt, ...)
Sparse is not happy to see non-static variable without declaration: lib/vsprintf.c:61:6: warning: symbol 'no_hash_pointers' was not declared. Should it be static? Declare respective variable in the local header. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- lib/test_printf.c | 4 ++-- lib/vsprintf.c | 1 + lib/vsprintf.h | 7 +++++++ mm/kfence/report.c | 3 +-- 4 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 lib/vsprintf.h