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

Signed-off-by: Daniel Guilak
---
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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Tue, 2008-07-08 at 14:21 -0700, Daniel Guilak wrote:
> Signed-off-by: Daniel Guilak

Looks reasonable.

Acked-by: Josh Triplett

> ---
> 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 = {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Tue, 08 Jul 2008 14:21:09 -0700 Daniel Guilak wrote:

> Signed-off-by: Daniel Guilak

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?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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
---
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
#include

+#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



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

On Fri, 2008-07-11 at 12:18 -0700, Andrew Morton wrote:
> On Tue, 08 Jul 2008 14:21:09 -0700 Daniel Guilak wrote:
>
> > Signed-off-by: Daniel Guilak
>
> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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
> ---
> 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
> #include
>
> +#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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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
> > ---
> > 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
> > #include
> >
> > +#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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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
> > > ---
> > > 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
> > > #include
> > >
> > > +#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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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
> > > ---
> > > 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
> > > #include
> > >
> > > +#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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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
> > > > ---
> > > > 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
> > > > #include
> > > >
> > > > +#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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/