2005-11-04 04:19:09

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] I8K: convert to seqfile

On Sat, Jun 25, 2005 at 06:03:50PM -0700, Linux Kernel wrote:
> tree e76bf5589246831604130349ae67b30b998deb29
> parent e70c9d5e61c6cb2272c866fc1303e62975006752
> author Dmitry Torokhov <[email protected]> Sun, 26 Jun 2005 04:54:26 -0700
> committer Linus Torvalds <[email protected]> Sun, 26 Jun 2005 06:24:24 -0700
>
> [PATCH] I8K: convert to seqfile
>
> I8K: Change proc code to use seq_file.
>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
>
> drivers/char/i8k.c | 64 ++++++++++++++++++-----------------------------------
> 1 files changed, 22 insertions(+), 42 deletions(-)

This took a while to notice somehow, but one of our Fedora users
upgraded from a 2.6.12 kernel to 2.6.14 today, and noticed
that his gkrellm segfaulted[1].

The reason is that we've subtley changed the format of /proc/i8k

Before:
1.0 A38 ? 54 -22 1 -22 79260 -1 2

After:
1.0 A38 52 -22 1 -22 77340 -1 2


The missing '?' field is puzzling though. Looking at the diff,
this should work. Is this a shortfalling of seq_file perhaps ?

Dave

[1] The i8k plugin for that thing is hurrendous btw, don't
look at it with a weak stomach. It does no sanity checking
on arguments at all, and assumes things will stay constant.
Little wonder it blows up when it runs out of things to strcpy()


2005-11-04 04:41:19

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] I8K: convert to seqfile

On Thu, Nov 03, 2005 at 11:19:02PM -0500, Dave Jones wrote:
> The missing '?' field is puzzling though. Looking at the diff,
> this should work. Is this a shortfalling of seq_file perhaps ?

dmi_get_system_info() returns "" instead of "?" now, apparently...

2005-11-04 05:40:34

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] I8K: convert to seqfile

On Thursday 03 November 2005 23:41, Al Viro wrote:
> On Thu, Nov 03, 2005 at 11:19:02PM -0500, Dave Jones wrote:
> > The missing '?' field is puzzling though. Looking at the diff,
> > this should work. Is this a shortfalling of seq_file perhaps ?
>
> dmi_get_system_info() returns "" instead of "?" now, apparently...
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

Oops! I have that field in my BIOS so I never noticed the problem...

I wonder if something like the patch below will fix it.

--
Dmitry

Signed-off-by: Dmitry Torokhov <[email protected]>
---

drivers/char/i8k.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)

Index: work/drivers/char/i8k.c
===================================================================
--- work.orig/drivers/char/i8k.c
+++ work/drivers/char/i8k.c
@@ -99,7 +99,9 @@ struct smm_regs {

static inline char *i8k_get_dmi_data(int field)
{
- return dmi_get_system_info(field) ? : "N/A";
+ char *dmi_data = dmi_get_system_info(field);
+
+ return dmi_data && *dmi_data ? dmi_data : "?";
}

/*