[lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[Uwe Hermann]

[-- Attachment #1.1: Type: text/plain, Size: 4896 bytes --]

Hi everyone,

I'd like to announce a tool called 'superiotool' which was developed by
a bunch of LinuxBIOS developers over at linuxbios.org:

  http://linuxbios.org/Superiotool

Superiotool is a user-space utility which can

 - detect which Super I/O chip is soldered onto your mainboard,

 - at which configuration port it's located (usually 0x2e or 0x4e), and

 - dump all register contents of the Super I/O chip, together with the
   default values as per datasheet (to make comparing the values easy).

Installation:

 $ svn co svn://linuxbios.org/repos/trunk/util/superiotool
 $ cd superiotool
 $ make
 $ sudo make install

It's written in C and licensed under the GPL (version 2 or later).

We're using it for LinuxBIOS development purposes (e.g. you need to
properly init the Super I/O for early serial output support), but I
guess this might be a useful tool for lm-sensors purposes, too.

Yes, we're aware of sensors-detect and we considered merging our
code into sensors-detect for a while, but decided to keep them two
different tools for now. Some reasons (or differenceѕ between the tools):

 - superiotool is written in C, which allows (some) code copying into
   LinuxBIOS (which is also written in C).

 - superiotool _only_ supports and cares about Super I/Os, not other
   sensors, I2C, etc. etc. We don't intend to support anything other
   than Super I/Os in superiotool (hence the name :-) We don't need
   sensors et al in LinuxBIOS, that's mostly handled by the OS/userspace,
   e.g. lm-sensors.

 - The main difference to the sensors-detect Super I/O support is in the
   register dump feature, I guess. You can use 'superiotool -d' to get
   output like this:

     $ superiotool -d
     Found Winbond W83627EHF/EF/EHG/EG (id=0x88, rev=0x63) at 0x2e
     Register dump:
     idx 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 
     val 88 63 ff 00 44 00 00 ff 50 04 00 00 9a 21 00 ff 
     def 88 MM ff 00 MM 00 MM RR 50 04 00 RR 00 21 00 00 
     LDN 0x00 (Floppy)
     idx 30 60 61 70 74 f0 f1 f2 f4 f5 
     val 00 03 f0 06 02 0e 00 ff 00 00 
     def 01 03 f0 06 02 8e 00 ff 00 00 
     LDN 0x01 (Parallel port)
     idx 30 60 61 70 74 f0 
     val 00 03 78 00 04 3c 
     def 01 03 78 07 04 3f 
     LDN 0x02 (COM1)
     idx 30 60 61 70 f0 
     val 00 03 f8 04 00 
     def 01 03 f8 04 00 
     LDN 0x03 (COM2)
     idx 30 60 61 70 f0 f1 
     val 00 02 f8 03 00 00 
     def 01 02 f8 03 00 00 
     LDN 0x05 (Keyboard)
     idx 30 60 61 62 63 70 72 f0 
     val 01 00 60 00 64 01 0c 83 
     def 01 00 60 00 64 01 0c 83 
     LDN 0x06 (Serial flash interface)
     idx 30 62 63 
     val 00 ff ff 
     def 00 00 00 
     LDN 0x07 (GPIO 1, GPIO 6, game port, MIDI port)
     idx 30 60 61 62 63 70 f0 f1 f2 f3 f4 f5 f6 f7 
     val 00 02 01 03 30 00 ff ff ff ff ff ff ff 00 
     def 00 02 01 03 30 09 ff 00 00 00 ff 00 00 00 
     LDN 0x08 (WDTO#, PLED)
     idx 30 f5 f6 f7 
     val 00 ff 00 ff 
     def 00 00 00 00 
     LDN 0x09 (GPIO 2, GPIO 3, GPIO 4, GPIO 5, SUSLED)
     idx 30 e0 e1 e2 e3 e4 e5 f0 f1 f2 f3 f4 f5 f6 f7 
     val 0f ff 21 00 ff 08 00 ff 0c 00 09 9f ff 00 00 
     def 00 ff 00 00 ff 00 00 ff 00 00 00 ff 00 00 00 
     LDN 0x0a (ACPI)
     idx 30 70 e0 e1 e2 e3 e4 e5 e6 e7 e8 f2 f3 f4 f6 f7 
     val 01 00 00 00 ff 00 40 02 0c 10 09 7d 00 00 00 00 
     def 00 00 01 00 ff 08 00 RR 00 00 RR 7c 00 00 00 00 
     LDN 0x0b (Hardware monitor)
     idx 30 60 61 70 f0 f1 
     val 01 02 90 00 c1 3f 
     def 00 00 00 00 c1 00

  All numbers are in hex. idx is the register number, val is the current
  content of the register, def is the default value as per datasheet.

  I assume the dump feature might help with lm-sensors development, as
  you can easily dump the hardware monitor register contents (or have
  users send dumps) and compare values from different boards/chips etc.


We currently detect a number of chips which are not detected by
sensors-detect (and vice versa), I'll provide patches to sensors-detect
to keep the number of detected Super I/Os in sync between the tools.

This is open-source of course, so feel free to poke in the code and reuse
whatever parts of it you need or want. Contributions to supertiotool are
welcome, too, of course :)

We also try to collect a number of example register dumps for various
chips in the LinuxBIOS wiki for reference (any contributions are welcome
here, too; just run 'superiotool -dV' and send the output to the
LinuxBIOS mailing list, see http://linuxbios.org/Mailinglist).

The list of currently supported chips, as well as links to sample dumps,
is available here: http://linuxbios.org/Superiotool.


HTH, Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[Jean Delvare]

Hi Uwe,

On Mon, 8 Oct 2007 03:35:40 +0200, Uwe Hermann wrote:
> I'd like to announce a tool called 'superiotool' which was developed by
> a bunch of LinuxBIOS developers over at linuxbios.org:
> 
>   http://linuxbios.org/Superiotool
> 
> Superiotool is a user-space utility which can
> 
>  - detect which Super I/O chip is soldered onto your mainboard,
> 
>  - at which configuration port it's located (usually 0x2e or 0x4e), and
> 
>  - dump all register contents of the Super I/O chip, together with the
>    default values as per datasheet (to make comparing the values easy).
> 
> Installation:
> 
>  $ svn co svn://linuxbios.org/repos/trunk/util/superiotool
>  $ cd superiotool
>  $ make
>  $ sudo make install
> 
> It's written in C and licensed under the GPL (version 2 or later).
> 
> We're using it for LinuxBIOS development purposes (e.g. you need to
> properly init the Super I/O for early serial output support), but I
> guess this might be a useful tool for lm-sensors purposes, too.

We already have sensors-detect for identification and isadump for
register dumps, so I'm not sure if superiotool will be very useful to
us in practice.

> Yes, we're aware of sensors-detect and we considered merging our
> code into sensors-detect for a while, but decided to keep them two
> different tools for now. Some reasons (or differenceѕ between the tools):
> 
>  - superiotool is written in C, which allows (some) code copying into
>    LinuxBIOS (which is also written in C).
> 
>  - superiotool _only_ supports and cares about Super I/Os, not other
>    sensors, I2C, etc. etc. We don't intend to support anything other
>    than Super I/Os in superiotool (hence the name :-) We don't need
>    sensors et al in LinuxBIOS, that's mostly handled by the OS/userspace,
>    e.g. lm-sensors.

I completely agree that superiotool and sensors-detect have to stay
separate tools. They fulfill different needs.

>  - The main difference to the sensors-detect Super I/O support is in the
>    register dump feature, I guess. You can use 'superiotool -d' to get
>    output like this:
> 
>      $ superiotool -d
>      Found Winbond W83627EHF/EF/EHG/EG (id=0x88, rev=0x63) at 0x2e
>      Register dump:
>      idx 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 
>      val 88 63 ff 00 44 00 00 ff 50 04 00 00 9a 21 00 ff 
>      def 88 MM ff 00 MM 00 MM RR 50 04 00 RR 00 21 00 00 
>      LDN 0x00 (Floppy)
>      idx 30 60 61 70 74 f0 f1 f2 f4 f5 
>      val 00 03 f0 06 02 0e 00 ff 00 00 
>      def 01 03 f0 06 02 8e 00 ff 00 00 
>      LDN 0x01 (Parallel port)
>      idx 30 60 61 70 74 f0 
>      val 00 03 78 00 04 3c 
>      def 01 03 78 07 04 3f 
>      LDN 0x02 (COM1)
>      idx 30 60 61 70 f0 
>      val 00 03 f8 04 00 
>      def 01 03 f8 04 00 
>      LDN 0x03 (COM2)
>      idx 30 60 61 70 f0 f1 
>      val 00 02 f8 03 00 00 
>      def 01 02 f8 03 00 00 
>      LDN 0x05 (Keyboard)
>      idx 30 60 61 62 63 70 72 f0 
>      val 01 00 60 00 64 01 0c 83 
>      def 01 00 60 00 64 01 0c 83 
>      LDN 0x06 (Serial flash interface)
>      idx 30 62 63 
>      val 00 ff ff 
>      def 00 00 00 
>      LDN 0x07 (GPIO 1, GPIO 6, game port, MIDI port)
>      idx 30 60 61 62 63 70 f0 f1 f2 f3 f4 f5 f6 f7 
>      val 00 02 01 03 30 00 ff ff ff ff ff ff ff 00 
>      def 00 02 01 03 30 09 ff 00 00 00 ff 00 00 00 
>      LDN 0x08 (WDTO#, PLED)
>      idx 30 f5 f6 f7 
>      val 00 ff 00 ff 
>      def 00 00 00 00 
>      LDN 0x09 (GPIO 2, GPIO 3, GPIO 4, GPIO 5, SUSLED)
>      idx 30 e0 e1 e2 e3 e4 e5 f0 f1 f2 f3 f4 f5 f6 f7 
>      val 0f ff 21 00 ff 08 00 ff 0c 00 09 9f ff 00 00 
>      def 00 ff 00 00 ff 00 00 ff 00 00 00 ff 00 00 00 
>      LDN 0x0a (ACPI)
>      idx 30 70 e0 e1 e2 e3 e4 e5 e6 e7 e8 f2 f3 f4 f6 f7 
>      val 01 00 00 00 ff 00 40 02 0c 10 09 7d 00 00 00 00 
>      def 00 00 01 00 ff 08 00 RR 00 00 RR 7c 00 00 00 00 
>      LDN 0x0b (Hardware monitor)
>      idx 30 60 61 70 f0 f1 
>      val 01 02 90 00 c1 3f 
>      def 00 00 00 00 c1 00
> 
>   All numbers are in hex. idx is the register number, val is the current
>   content of the register, def is the default value as per datasheet.
> 
>   I assume the dump feature might help with lm-sensors development, as
>   you can easily dump the hardware monitor register contents (or have
>   users send dumps) and compare values from different boards/chips etc.

We tell users to use isadump for that. As isadump is part of the
lm-sensors package, we can be reasonably certain that it is available
to the users. And more often than not, we need the dump of the
LDN-specific I/O space, which superio doesn't support as far as I can
see.

> We currently detect a number of chips which are not detected by
> sensors-detect (and vice versa), I'll provide patches to sensors-detect
> to keep the number of detected Super I/Os in sync between the tools.
>
> This is open-source of course, so feel free to poke in the code and reuse
> whatever parts of it you need or want. Contributions to supertiotool are
> welcome, too, of course :)

Thanks you. I'm all for sharing the information.

Two things I've noticed while trying superiotool:

* The human-readable dump of my F71805FG chip includes:
27 
29\0
2aÿ
2bÿ
That's not exactly human-readable by my definition ;)

* When dumping a Winbond W83627THF chip, sometimes the default value is
replaced with NA, MM or RR. While I can imagine what NA stands for, I
am curious what MM and RR mean. This should be explained in the manual
page IMHO.

I've also hit a strange bug with -v (no kidding), I'll send a patch
when I'm done fixing it.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[Uwe Hermann]

[-- Attachment #1.1: Type: text/plain, Size: 3464 bytes --]

Hi,

On Tue, Oct 09, 2007 at 07:14:29PM +0200, Jean Delvare wrote:
> >      LDN 0x0a (ACPI)
> >      idx 30 70 e0 e1 e2 e3 e4 e5 e6 e7 e8 f2 f3 f4 f6 f7 
> >      val 01 00 00 00 ff 00 40 02 0c 10 09 7d 00 00 00 00 
> >      def 00 00 01 00 ff 08 00 RR 00 00 RR 7c 00 00 00 00 
> >      LDN 0x0b (Hardware monitor)
> >      idx 30 60 61 70 f0 f1 
> >      val 01 02 90 00 c1 3f 
> >      def 00 00 00 00 c1 00
> > 
> >   All numbers are in hex. idx is the register number, val is the current
> >   content of the register, def is the default value as per datasheet.
> > 
> >   I assume the dump feature might help with lm-sensors development, as
> >   you can easily dump the hardware monitor register contents (or have
> >   users send dumps) and compare values from different boards/chips etc.
> 
> We tell users to use isadump for that. As isadump is part of the
> lm-sensors package, we can be reasonably certain that it is available
> to the users. And more often than not, we need the dump of the
> LDN-specific I/O space, which superio doesn't support as far as I can see.

It dumps the per-LDN register contents, not sure if that's the same as
the I/O space you're referring to? If no, then it's not supported, correct.


> Two things I've noticed while trying superiotool:
> 
> * The human-readable dump of my F71805FG chip includes:
> 27=20
> 29=00
> 2a=ff
> 2b=ff
> That's not exactly human-readable by my definition ;)

Yeah, that was the initial format in the very early days, the plain
register dump feature came later and we redeclared the old format
to be the "human-readable" format for now (but it really needs lots of
improvements, and isn't available for all chips anyway).

This is likely a _very_ chip-specific feature, so we need one extra
function with tons of custom printf's per chip. I'm not too sure if
this is really _that_ useful to warrant such an amount of work, but
we'll see...


> * When dumping a Winbond W83627THF chip, sometimes the default value is
> replaced with NA, MM or RR. While I can imagine what NA stands for, I
> am curious what MM and RR mean. This should be explained in the manual
> page IMHO.

Yes, the documentation needs improvements.

NA = None available (refers to the default register value from the datasheet)
RR = Reserved (register is marked as "Reserved" in the datasheet)
MM = Misc.

The NA vs. MM usage is not consistent at the moment. MM is meant for
default values which cannot be expressed as a simple hex number
(stuff like "Bit 3 is 1 if SYSCF is pulled-up, bit 7 is 0 except
on full moons, the remaining bits are 1").

Ideally, we'll implement a sort of "footnotes" for MM entries where
the specifics are explained (in the superiotool output), but that's not
possible, yet. It's also debatable whether it's useful to do that work
if you can just look into the respective datasheet for MM values.


> I've also hit a strange bug with -v (no kidding), I'll send a patch
> when I'm done fixing it.

Yeah, known issue (if we're talking about the same one). See the thread
starting here http://linuxbios.org/pipermail/linuxbios/2007-October/025461.html
for a discussion and possible solutions. We haven't yet found the best
way to do it, but will have to choose one soonish.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[Jean Delvare]

On Tue, 9 Oct 2007 19:54:24 +0200, Uwe Hermann wrote:
> On Tue, Oct 09, 2007 at 07:14:29PM +0200, Jean Delvare wrote:
> > We tell users to use isadump for that. As isadump is part of the
> > lm-sensors package, we can be reasonably certain that it is available
> > to the users. And more often than not, we need the dump of the
> > LDN-specific I/O space, which superio doesn't support as far as I can see.
> 
> It dumps the per-LDN register contents, not sure if that's the same as
> the I/O space you're referring to? If no, then it's not supported, correct.

No, that's not the same I/O space. What I'm talking about is the I/O
space those address is typically stored in registers 0x60 and 0x61 for
a given logical device.

> > * The human-readable dump of my F71805FG chip includes:
> > 27 
> > 29\0
> > 2aÿ
> > 2bÿ
> > That's not exactly human-readable by my definition ;)
> 
> Yeah, that was the initial format in the very early days, the plain
> register dump feature came later and we redeclared the old format
> to be the "human-readable" format for now (but it really needs lots of
> improvements, and isn't available for all chips anyway).
> 
> This is likely a _very_ chip-specific feature, so we need one extra
> function with tons of custom printf's per chip. I'm not too sure if
> this is really _that_ useful to warrant such an amount of work, but
> we'll see...

I don't know if it's worth the effort, but if not, then you should
probably not print these values at all. They're available in the other
dump format anyway.

> > I've also hit a strange bug with -v (no kidding), I'll send a patch
> > when I'm done fixing it.
> 
> Yeah, known issue (if we're talking about the same one). See the thread
> starting here http://linuxbios.org/pipermail/linuxbios/2007-October/025461.html
> for a discussion and possible solutions. We haven't yet found the best
> way to do it, but will have to choose one soonish.

No, I didn't go that far. My concern was simply that strncpy is
misused. strncpy doesn't null-terminate the string it copies, so you
end up passing an invalid string to printf.

I have a simple fix using atoi instead of the ugly strncpy/strlen mix
you have:

 superiotool.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

--- superiotool.orig/superiotool.c	2007-10-09 19:48:15.000000000 +0200
+++ superiotool/superiotool.c	2007-10-09 19:53:43.000000000 +0200
@@ -170,12 +170,7 @@ void probing_for(const char *vendor, con
 
 static void print_version(void)
 {
-	char tmp[80];
-
-	strncpy((char *)&tmp,
-		(const char *)&SUPERIOTOOL_VERSION[6],
-		strlen(SUPERIOTOOL_VERSION) - 8);
-	printf("superiotool r%s\n", (char *)&tmp);
+	printf("superiotool r%d\n", atoi(&SUPERIOTOOL_VERSION[6]));
 }
 
 int main(int argc, char *argv[])

But of course it will conflict with the patch you posted.

I don't like your patch at all, BTW. At this point of complexity you
have to admit that you're doing something wrong and give up ;) Why
don't you use arbitrary version numbers as every other projects do?
This is much more valuable than SVN revision numbers, and much easier
to maintain than what you're trying to do.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[Uwe Hermann]

[-- Attachment #1.1: Type: text/plain, Size: 4341 bytes --]

On Tue, Oct 09, 2007 at 08:22:54PM +0200, Jean Delvare wrote:
> > It dumps the per-LDN register contents, not sure if that's the same as
> > the I/O space you're referring to? If no, then it's not supported, correct.
> 
> No, that's not the same I/O space. What I'm talking about is the I/O
> space those address is typically stored in registers 0x60 and 0x61 for
> a given logical device.

Ah, ok. That's not printed by superiotool. Maybe we'll add that later
(but I don't think we need it for LinuxBIOS purposes, so we probably won't).


> > This is likely a _very_ chip-specific feature, so we need one extra
> > function with tons of custom printf's per chip. I'm not too sure if
> > this is really _that_ useful to warrant such an amount of work, but
> > we'll see...
> 
> I don't know if it's worth the effort, but if not, then you should
> probably not print these values at all. They're available in the other
> dump format anyway.

Yes, if at all, the human-readable output should only print readable text.


> > > I've also hit a strange bug with -v (no kidding), I'll send a patch
> > > when I'm done fixing it.
> > 
> > Yeah, known issue (if we're talking about the same one). See the thread
> > starting here http://linuxbios.org/pipermail/linuxbios/2007-October/025461.html
> > for a discussion and possible solutions. We haven't yet found the best
> > way to do it, but will have to choose one soonish.
> 
> No, I didn't go that far. My concern was simply that strncpy is
> misused. strncpy doesn't null-terminate the string it copies, so you
> end up passing an invalid string to printf.

Ah, yes, that's likely the reason for random junk the the end of the
version number some people have noticed.


> I have a simple fix using atoi instead of the ugly strncpy/strlen mix
> you have:
> 
>  superiotool.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> --- superiotool.orig/superiotool.c	2007-10-09 19:48:15.000000000 +0200
> +++ superiotool/superiotool.c	2007-10-09 19:53:43.000000000 +0200
> @@ -170,12 +170,7 @@ void probing_for(const char *vendor, con
>  
>  static void print_version(void)
>  {
> -	char tmp[80];
> -
> -	strncpy((char *)&tmp,
> -		(const char *)&SUPERIOTOOL_VERSION[6],
> -		strlen(SUPERIOTOOL_VERSION) - 8);
> -	printf("superiotool r%s\n", (char *)&tmp);
> +	printf("superiotool r%d\n", atoi(&SUPERIOTOOL_VERSION[6]));

Yeah, that was my first thought, but IIRC atoi() is deprecated in favor
of strotol(), or at least it doesn't detect any errors (according to the
manpage), which strtol() does. Btw, atoi() has to ignore random non-digit
junk at the end of the string, is it guaranteed to do that in all
implementations (different libc's or platforms, e.g. *BSD, Solaris, etc)?

Also, _if_ the svn revision number gets too big, atoi()/strtol() will
"truncate" it (yes, this is more a theoretical issue, won't happen
anytime soon).


> I don't like your patch at all, BTW. At this point of complexity you
> have to admit that you're doing something wrong and give up ;) Why

Nah, never give up :)


> don't you use arbitrary version numbers as every other projects do?

We don't do tarball releases, so arbitrary 0.x version numbers aren't
that useful, and...


> This is much more valuable than SVN revision numbers, and much easier
> to maintain than what you're trying to do.

...we want svn revisions as version number in order to be able to
exactly match a bug to a certain svn revision (and also to match dump
outputs to svn revisions).

Svn revision numbers are (if the solution works correctly) completely
maintenance-free, you don't have to change a single line of code, and
the number will be increased automatically upon every commit. A lot
easier than "manual" version number updates (which you'll forget often).

FWIW, it looks like this proposal is the best so far, so we'll probably
use it: http://linuxbios.org/pipermail/linuxbios/2007-October/025551.html
It gathers the svn version at build time (thus makes 'svnversion' a
build requirement, but we can live with that). Other than that it does
all we want, I think.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [ANNOUNCE] superiotool -- Detect Super I/Os,

[Jean Delvare]

On Tue, 9 Oct 2007 21:16:22 +0200, Uwe Hermann wrote:
> On Tue, Oct 09, 2007 at 08:22:54PM +0200, Jean Delvare wrote:
> > No, I didn't go that far. My concern was simply that strncpy is
> > misused. strncpy doesn't null-terminate the string it copies, so you
> > end up passing an invalid string to printf.
> 
> Ah, yes, that's likely the reason for random junk the the end of the
> version number some people have noticed.

Yes it is.

> > I have a simple fix using atoi instead of the ugly strncpy/strlen mix
> > you have:
> > 
> >  superiotool.c |    7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > --- superiotool.orig/superiotool.c	2007-10-09 19:48:15.000000000 +0200
> > +++ superiotool/superiotool.c	2007-10-09 19:53:43.000000000 +0200
> > @@ -170,12 +170,7 @@ void probing_for(const char *vendor, con
> >  
> >  static void print_version(void)
> >  {
> > -	char tmp[80];
> > -
> > -	strncpy((char *)&tmp,
> > -		(const char *)&SUPERIOTOOL_VERSION[6],
> > -		strlen(SUPERIOTOOL_VERSION) - 8);
> > -	printf("superiotool r%s\n", (char *)&tmp);
> > +	printf("superiotool r%d\n", atoi(&SUPERIOTOOL_VERSION[6]));
> 
> Yeah, that was my first thought, but IIRC atoi() is deprecated in favor
> of strotol(), or at least it doesn't detect any errors (according to the
> manpage), which strtol() does. Btw, atoi() has to ignore random non-digit

No, atoi() isn't deprecated. It indeed doesn't detect errors, but OTOH
what kind of error do you expect, and how would you handle it?

> junk at the end of the string, is it guaranteed to do that in all
> implementations (different libc's or platforms, e.g. *BSD, Solaris, etc)?

I think so, but I haven't tested it.

> Also, _if_ the svn revision number gets too big, atoi()/strtol() will
> "truncate" it (yes, this is more a theoretical issue, won't happen
> anytime soon).

You mean, if the svn revision exceeds 2147483647? Come on.

> > don't you use arbitrary version numbers as every other projects do?
> 
> We don't do tarball releases, so arbitrary 0.x version numbers aren't
> that useful, and...

Doing no tarball releases is probably not a good idea to start with.
This will give your tool a bad visibility. Human-friendly version
numbers give packagers an idea of what changes are going in. With SVN
revision numbers, you can't tell a maintenance update from a complete
rewrite.

> > This is much more valuable than SVN revision numbers, and much easier
> > to maintain than what you're trying to do.
> 
> ...we want svn revisions as version number in order to be able to
> exactly match a bug to a certain svn revision (and also to match dump
> outputs to svn revisions).
> 
> Svn revision numbers are (if the solution works correctly) completely
> maintenance-free, you don't have to change a single line of code, and

Maintenance-free until you add a new source file to the project.

> the number will be increased automatically upon every commit. A lot
> easier than "manual" version number updates (which you'll forget often).
> 
> FWIW, it looks like this proposal is the best so far, so we'll probably
> use it: http://linuxbios.org/pipermail/linuxbios/2007-October/025551.html
> It gathers the svn version at build time (thus makes 'svnversion' a
> build requirement, but we can live with that). Other than that it does
> all we want, I think.

I had been thinking about this, yes. If you really want to use the SVN
revision number as the version, and if you really don't want to release
tarballs of your project, then that's the way to go.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors