[PATCH] init/version.c: Silenced sparse warning by declaring the version string.

[PATCH] init/version.c: Silenced sparse warning by declaring the version string.

[Daniel Guilak]

Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
---
 init/version.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/init/version.c b/init/version.c
index 9d17d70..041fd82 100644
--- a/init/version.c
+++ b/init/version.c
@@ -16,6 +16,7 @@
 #define version(a) Version_ ## a
 #define version_string(a) version(a)
 
+extern int version_string(LINUX_VERSION_CODE);
 int version_string(LINUX_VERSION_CODE);
 
 struct uts_namespace init_uts_ns = {
-- 
1.5.4.3

Re: [PATCH] init/version.c: Silenced sparse warning by declaring the version string.

[Josh Triplett]

On Tue, 2008-07-08 at 14:21 -0700, Daniel Guilak wrote:
> Signed-off-by: Daniel Guilak <daniel@danielguilak.com>

Looks reasonable.

Acked-by: Josh Triplett <josh@kernel.org>

> ---
>  init/version.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/init/version.c b/init/version.c
> index 9d17d70..041fd82 100644
> --- a/init/version.c
> +++ b/init/version.c
> @@ -16,6 +16,7 @@
>  #define version(a) Version_ ## a
>  #define version_string(a) version(a)
> 
> +extern int version_string(LINUX_VERSION_CODE);
>  int version_string(LINUX_VERSION_CODE);
> 
>  struct uts_namespace init_uts_ns = {

Re: [PATCH] init/version.c: Silenced sparse warning by declaring the version string.

[Andrew Morton]

On Tue, 08 Jul 2008 14:21:09 -0700 Daniel Guilak <guilak@linux.vnet.ibm.com> wrote:

> Signed-off-by: Daniel Guilak <daniel@danielguilak.com>

Please always quote the warning or error message in the changelog when
fixing it.  Although it's pretty obvious in this case.

>  init/version.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/init/version.c b/init/version.c
> index 9d17d70..041fd82 100644
> --- a/init/version.c
> +++ b/init/version.c
> @@ -16,6 +16,7 @@
>  #define version(a) Version_ ## a
>  #define version_string(a) version(a)
>  
> +extern int version_string(LINUX_VERSION_CODE);
>  int version_string(LINUX_VERSION_CODE);
>  
>  struct uts_namespace init_uts_ns = {

hrm, what does this thing do?  Seems to define 

int Version_132634;

Then sticks that in the symbol table (and wastes a bit of bss).

Does anything use it?

Could it be made static?

Re: [PATCH] init/version.c: Silenced sparse warning by declaring the version string.

[Daniel Guilak]

On Fri, 2008-07-11 at 12:18 -0700, Andrew Morton wrote:
> On Tue, 08 Jul 2008 14:21:09 -0700 Daniel Guilak <guilak@linux.vnet.ibm.com> wrote:
> 
> > Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
> 
> Please always quote the warning or error message in the changelog when
> fixing it.  Although it's pretty obvious in this case.
> 
> >  init/version.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/init/version.c b/init/version.c
> > index 9d17d70..041fd82 100644
> > --- a/init/version.c
> > +++ b/init/version.c
> > @@ -16,6 +16,7 @@
> >  #define version(a) Version_ ## a
> >  #define version_string(a) version(a)
> >  
> > +extern int version_string(LINUX_VERSION_CODE);
> >  int version_string(LINUX_VERSION_CODE);
> >  
> >  struct uts_namespace init_uts_ns = {
> 
> hrm, what does this thing do?  Seems to define 
> 
> int Version_132634;
> 
> Then sticks that in the symbol table (and wastes a bit of bss).
> 
> Does anything use it?
> 
> Could it be made static?
> 
> 
Apparently it's only used by the ksymoops tool, which is not needed if
(according to README) the kernel is configured with CONFIG_KALLSYMS.

So I will submit a patch depending on this one that will define
version_string only if CONFIG_KALLSYMS is not defined, and hopefully
that will deal with the problem.

--Daniel Guilak

[PATCH] init/version.c: Define version_string only if CONFIG_KALLSYMS is not defined.

[Daniel Guilak]

int Version_* is only used with ksymoops, which is only needed (according to
README and Documentation/Changes) if CONFIG_KALLSYMS is NOT defined.  Therefore
this patch defines version_string only if CONFIG_KALLSYMS is not defined.

Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
---
 init/version.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Depends on "[PATCH] init/version.c: Silenced sparse warning by declaring the version string."

diff --git a/init/version.c b/init/version.c
index 041fd82..52a8b98 100644
--- a/init/version.c
+++ b/init/version.c
@@ -13,11 +13,13 @@
 #include <linux/utsrelease.h>
 #include <linux/version.h>
 
+#ifndef CONFIG_KALLSYMS
 #define version(a) Version_ ## a
 #define version_string(a) version(a)
 
 extern int version_string(LINUX_VERSION_CODE);
 int version_string(LINUX_VERSION_CODE);
+#endif
 
 struct uts_namespace init_uts_ns = {
 	.kref = {
-- 
1.5.4.3

Re: [PATCH] init/version.c: Define version_string only if CONFIG_KALLSYMS is not defined.

[Randy Dunlap]

On Mon, 14 Jul 2008 12:04:46 -0700 Daniel Guilak wrote:

> int Version_* is only used with ksymoops, which is only needed (according to
> README and Documentation/Changes) if CONFIG_KALLSYMS is NOT defined.  Therefore
> this patch defines version_string only if CONFIG_KALLSYMS is not defined.
> 
> Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
> ---
>  init/version.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> Depends on "[PATCH] init/version.c: Silenced sparse warning by declaring the version string."
> 
> diff --git a/init/version.c b/init/version.c
> index 041fd82..52a8b98 100644
> --- a/init/version.c
> +++ b/init/version.c
> @@ -13,11 +13,13 @@
>  #include <linux/utsrelease.h>
>  #include <linux/version.h>
>  
> +#ifndef CONFIG_KALLSYMS
>  #define version(a) Version_ ## a
>  #define version_string(a) version(a)
>  
>  extern int version_string(LINUX_VERSION_CODE);
>  int version_string(LINUX_VERSION_CODE);
> +#endif
>  
>  struct uts_namespace init_uts_ns = {
>  	.kref = {
> -- 

Does not apply cleanly to linux-2.6.26, linux-next-20080714, or Linus-git-head.

and why do we ned both extern int version_string()
and int version_string() ?

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

Re: [PATCH] init/version.c: Define version_string only if CONFIG_KALLSYMS is not defined.

[Daniel Guilak]

On Mon, 2008-07-14 at 12:38 -0700, Randy Dunlap wrote:
> On Mon, 14 Jul 2008 12:04:46 -0700 Daniel Guilak wrote:
> 
> > int Version_* is only used with ksymoops, which is only needed (according to
> > README and Documentation/Changes) if CONFIG_KALLSYMS is NOT defined.  Therefore
> > this patch defines version_string only if CONFIG_KALLSYMS is not defined.
> > 
> > Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
> > ---
> >  init/version.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > Depends on "[PATCH] init/version.c: Silenced sparse warning by declaring the version string."
> > 
> > diff --git a/init/version.c b/init/version.c
> > index 041fd82..52a8b98 100644
> > --- a/init/version.c
> > +++ b/init/version.c
> > @@ -13,11 +13,13 @@
> >  #include <linux/utsrelease.h>
> >  #include <linux/version.h>
> >  
> > +#ifndef CONFIG_KALLSYMS
> >  #define version(a) Version_ ## a
> >  #define version_string(a) version(a)
> >  
> >  extern int version_string(LINUX_VERSION_CODE);
> >  int version_string(LINUX_VERSION_CODE);
> > +#endif
> >  
> >  struct uts_namespace init_uts_ns = {
> >  	.kref = {
> > -- 
> 
> Does not apply cleanly to linux-2.6.26, linux-next-20080714, or Linus-git-head.
> 

Did you apply "[PATCH] init/version.c: Silenced sparse warning by
declaring the version string." beforehand? I just applied it on the most
recent tree and I didn't have any issues.

> and why do we ned both extern int version_string()
> and int version_string() ?
Because sparse was complaining that it wasn't prototyped or static.
There isn't a header file to put it in since it only exists to be a
kernel.
> ---
> ~Randy
> Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
> http://linuxplumbersconf.org/

--Daniel

Re: [PATCH] init/version.c: Define version_string only if CONFIG_KALLSYMS is not defined.

[Daniel Guilak]

On Mon, 2008-07-14 at 14:20 -0700, Daniel Guilak wrote:
> On Mon, 2008-07-14 at 12:38 -0700, Randy Dunlap wrote:
> > On Mon, 14 Jul 2008 12:04:46 -0700 Daniel Guilak wrote:
> > 
> > > int Version_* is only used with ksymoops, which is only needed (according to
> > > README and Documentation/Changes) if CONFIG_KALLSYMS is NOT defined.  Therefore
> > > this patch defines version_string only if CONFIG_KALLSYMS is not defined.
> > > 
> > > Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
> > > ---
> > >  init/version.c |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > Depends on "[PATCH] init/version.c: Silenced sparse warning by declaring the version string."
> > > 
> > > diff --git a/init/version.c b/init/version.c
> > > index 041fd82..52a8b98 100644
> > > --- a/init/version.c
> > > +++ b/init/version.c
> > > @@ -13,11 +13,13 @@
> > >  #include <linux/utsrelease.h>
> > >  #include <linux/version.h>
> > >  
> > > +#ifndef CONFIG_KALLSYMS
> > >  #define version(a) Version_ ## a
> > >  #define version_string(a) version(a)
> > >  
> > >  extern int version_string(LINUX_VERSION_CODE);
> > >  int version_string(LINUX_VERSION_CODE);
> > > +#endif
> > >  
> > >  struct uts_namespace init_uts_ns = {
> > >  	.kref = {
> > > -- 
> > 
> > Does not apply cleanly to linux-2.6.26, linux-next-20080714, or Linus-git-head.
> > 
> 
> Did you apply "[PATCH] init/version.c: Silenced sparse warning by
> declaring the version string." beforehand? I just applied it on the most
> recent tree and I didn't have any issues.
> 
> > and why do we ned both extern int version_string()
> > and int version_string() ?
> Because sparse was complaining that it wasn't prototyped or static.
> There isn't a header file to put it in since it only exists to be a
> kernel.

"Symbol", not "kernel", sorry.

> > ---
> > ~Randy
> > Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
> > http://linuxplumbersconf.org/
> 
> --Daniel

Re: [PATCH] init/version.c: Define version_string only if CONFIG_KALLSYMS is not defined.

[Randy Dunlap]

On Mon, 14 Jul 2008 14:20:38 -0700 Daniel Guilak wrote:

> On Mon, 2008-07-14 at 12:38 -0700, Randy Dunlap wrote:
> > On Mon, 14 Jul 2008 12:04:46 -0700 Daniel Guilak wrote:
> > 
> > > int Version_* is only used with ksymoops, which is only needed (according to
> > > README and Documentation/Changes) if CONFIG_KALLSYMS is NOT defined.  Therefore
> > > this patch defines version_string only if CONFIG_KALLSYMS is not defined.
> > > 
> > > Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
> > > ---
> > >  init/version.c |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > Depends on "[PATCH] init/version.c: Silenced sparse warning by declaring the version string."
> > > 
> > > diff --git a/init/version.c b/init/version.c
> > > index 041fd82..52a8b98 100644
> > > --- a/init/version.c
> > > +++ b/init/version.c
> > > @@ -13,11 +13,13 @@
> > >  #include <linux/utsrelease.h>
> > >  #include <linux/version.h>
> > >  
> > > +#ifndef CONFIG_KALLSYMS
> > >  #define version(a) Version_ ## a
> > >  #define version_string(a) version(a)
> > >  
> > >  extern int version_string(LINUX_VERSION_CODE);
> > >  int version_string(LINUX_VERSION_CODE);
> > > +#endif
> > >  
> > >  struct uts_namespace init_uts_ns = {
> > >  	.kref = {
> > > -- 
> > 
> > Does not apply cleanly to linux-2.6.26, linux-next-20080714, or Linus-git-head.
> > 
> 
> Did you apply "[PATCH] init/version.c: Silenced sparse warning by
> declaring the version string." beforehand? I just applied it on the most
> recent tree and I didn't have any issues.

Clearly I didn't apply that patch.  I missed anything saying that this was
patch 2/2 in a dependent series.

> > and why do we ned both extern int version_string()
> > and int version_string() ?
> Because sparse was complaining that it wasn't prototyped or static.
> There isn't a header file to put it in since it only exists to be a
> kernel.

so the extern makes it be published in the symbol table, whereas a static
would keep it private.  Is that correct?

and the second line (int version_string(LINUX_VERSION_CODE)) actually
defines it.

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

Re: [PATCH] init/version.c: Define version_string only if CONFIG_KALLSYMS is not defined.

[Daniel Guilak]

On Mon, 2008-07-14 at 15:07 -0700, Randy Dunlap wrote:
> On Mon, 14 Jul 2008 14:20:38 -0700 Daniel Guilak wrote:
> 
> > On Mon, 2008-07-14 at 12:38 -0700, Randy Dunlap wrote:
> > > On Mon, 14 Jul 2008 12:04:46 -0700 Daniel Guilak wrote:
> > > 
> > > > int Version_* is only used with ksymoops, which is only needed (according to
> > > > README and Documentation/Changes) if CONFIG_KALLSYMS is NOT defined.  Therefore
> > > > this patch defines version_string only if CONFIG_KALLSYMS is not defined.
> > > > 
> > > > Signed-off-by: Daniel Guilak <daniel@danielguilak.com>
> > > > ---
> > > >  init/version.c |    2 ++
> > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > 
> > > > Depends on "[PATCH] init/version.c: Silenced sparse warning by declaring the version string."
> > > > 
> > > > diff --git a/init/version.c b/init/version.c
> > > > index 041fd82..52a8b98 100644
> > > > --- a/init/version.c
> > > > +++ b/init/version.c
> > > > @@ -13,11 +13,13 @@
> > > >  #include <linux/utsrelease.h>
> > > >  #include <linux/version.h>
> > > >  
> > > > +#ifndef CONFIG_KALLSYMS
> > > >  #define version(a) Version_ ## a
> > > >  #define version_string(a) version(a)
> > > >  
> > > >  extern int version_string(LINUX_VERSION_CODE);
> > > >  int version_string(LINUX_VERSION_CODE);
> > > > +#endif
> > > >  
> > > >  struct uts_namespace init_uts_ns = {
> > > >  	.kref = {
> > > > -- 
> > > 
> > > Does not apply cleanly to linux-2.6.26, linux-next-20080714, or Linus-git-head.
> > > 
> > 
> > Did you apply "[PATCH] init/version.c: Silenced sparse warning by
> > declaring the version string." beforehand? I just applied it on the most
> > recent tree and I didn't have any issues.
> 
> Clearly I didn't apply that patch.  I missed anything saying that this was
> patch 2/2 in a dependent series.

Sorry about that, I should have been a bit more clear with it. Next
time I'll make sure to state that in the subject line. I stated the
dependency right before the diff message.

> > > and why do we ned both extern int version_string()
> > > and int version_string() ?
> > Because sparse was complaining that it wasn't prototyped or static.
> > There isn't a header file to put it in since it only exists to be a
> > kernel.
> 
> so the extern makes it be published in the symbol table, whereas a static
> would keep it private.  Is that correct?
> 
> and the second line (int version_string(LINUX_VERSION_CODE)) actually
> defines it.

Yes, that's exactly the idea behind it.

--Daniel