Re: [PATCH] I8K: convert to seqfile

Re: [PATCH] I8K: convert to seqfile

[Dave Jones]

On Sat, Jun 25, 2005 at 06:03:50PM -0700, Linux Kernel wrote:
 > tree e76bf5589246831604130349ae67b30b998deb29
 > parent e70c9d5e61c6cb2272c866fc1303e62975006752
 > author Dmitry Torokhov <dtor_core@ameritech.net> Sun, 26 Jun 2005 04:54:26 -0700
 > committer Linus Torvalds <torvalds@ppc970.osdl.org> 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 <dtor@mail.ru>
 > Signed-off-by: Andrew Morton <akpm@osdl.org>
 > Signed-off-by: Linus Torvalds <torvalds@osdl.org>
 > 
 >  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()

Re: [PATCH] I8K: convert to seqfile

[Al Viro]

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...

Re: [PATCH] I8K: convert to seqfile

[Dmitry Torokhov]

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 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/
> 

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 <dtor@mail.ru>
---

 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 : "?";
 }
 
 /*