Hi Lubomir,
On Sat, 30 May 2015 13:03:32 +0200, Lubomir Rintel wrote:
> ---
> eeprom/decode-dimms | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/eeprom/decode-dimms b/eeprom/decode-dimms
> index 3a50c07..4a861bd 100755
> --- a/eeprom/decode-dimms
> +++ b/eeprom/decode-dimms
> @@ -345,7 +345,7 @@ sub manufacturer_ddr3($$)
> my $manufacturer;
>
> return "Invalid" if parity($code) != 1;
> - return "Unknown" if ($code & 0x7F) - 1 > $vendors[$count & 0x7F];
> + return "Unknown" if ($code & 0x7F) > @{$vendors[$count & 0x7F]};
> $manufacturer = $vendors[$count & 0x7F][($code & 0x7F) - 1];
> $manufacturer =~ s/ \(former .*\)$// if $opt_side_by_side;
> $manufacturer .= "? (Invalid parity)" if parity($count) != 1;
Good catch, not sure how we managed to not notice this bug for years
despite the surrounding code being modified several times. Did you find
it by code inspection or did you actually hit the bug on one of your
memory modules?
Note that I would prefer
... if ($code & 0x7F) - 1 >= @{$vendors[$count & 0x7F]};
It is equivalent but more obviously correct as it matches the array
access on the following line.
Also, even after your fix, I think the code is not completely safe, as
we still don't check for ($code & 0x7F) being 0, nor do we verify that
$count & 0x7F does not exceed the number of elements in @vendors. I'll
fix that.
Thanks,
--
Jean Delvare
SUSE L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
