Salut Aurelien,
On Sat, 14 Jul 2012 17:26:26 +0200, Aurelien Jarno wrote:
> decode-dimms doesn't handle correctly the case where the eeprom module
> is loaded, but not eeprom is actually found (most of the cases when no
> bus i2c driver exist for the system):
>
> | # decode-dimms version 5929 (2011-02-16 14:58:38 +0100)
> |
> | Memory Serial Presence Detect Decoder
> | By Philip Edelbrock, Christian Zuckschwerdt, Burkart Lingner,
> | Jean Delvare, Trent Piepho and others
> | Can't use string ("") as a HASH ref while "strict refs" in use at /usr/bin/decode-dimms line 178
Line number here is incorrect, I suspect the trailing digit is missing.
> This tends to be a cryptic error message, at least for the users.
Obviously it isn't an error message for the user, it is a bug in the
script. I am able to reproduce this bug.
> The
> patch below fixes the issue by outputting an error message in that
> case.
>
> Index: eeprom/decode-dimms
> ===================================================================
> --- eeprom/decode-dimms (révision 6060)
> +++ eeprom/decode-dimms (copie de travail)
> @@ -1774,6 +1774,9 @@
> } elsif (! -d '/sys/module/eeprom') {
> print "No EEPROM found, are you sure the eeprom module is loaded?\n";
> exit;
> + } else {
> + print "No EEPROM found, the kernel probably does not support your hardware.\n";
> + exit;
> }
> }
The above message is speculation, there can be other reasons (for
example the system may have no SPD EEPROM at all, or the OS is not
given access to these.)
The bug is that function get_dimm_list returns random crap in the
problematic case (lack of return statement) instead of an empty array
as it should. I'll take the blame for it as it was introduced by a
cleanup commit of mine:
http://www.lm-sensors.org/changeset/5690
So IMO the proper fix would rather be:
--- i2c-tools.orig/eeprom/decode-dimms 2012-04-19 09:40:01.000000000 +0200
+++ i2c-tools/eeprom/decode-dimms 2012-07-15 19:17:38.687810333 +0200
@@ -1769,12 +1769,12 @@ sub get_dimm_list
close(DIR);
}
- if (@files) {
- return sort { $a->{file} cmp $b->{file} } @files;
- } elsif (! -d '/sys/module/eeprom') {
+ if (!@files && !-d '/sys/module/eeprom') {
print "No EEPROM found, are you sure the eeprom module is loaded?\n";
exit;
}
+
+ return sort { $a->{file} cmp $b->{file} } @files;
}
# @dimm is a list of hashes. There's one hash for each EEPROM we found.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
