+ if (name[0] >= '0' && name[0] <= '9') {
+ pin_nr = simple_strtoul(name, (char **)&name, 10);
+ if (name[0])
+ return -EINVAL;
Why do we need this check ? We have already copied / converted the
number in pin_nr, we do not need to check in the string again. And you
do not need to update name, because you are returning in any case.
This check checks if name pointer - after it's updated by simple_strtoul -
points to the terminating NULL character. If if does, name contained a valid
number. If it doesn't, it's pointing to trailing garbage and hence name didn't
contain a valid number.
In other words; This checks prevents a string like 3kjh;khji being interpreted
as 3.
But still you should check for (name != NULL) before accessing with
(name[0] >= '0' && name[0] <= '9')
Didn't do that because it's a static function. But since we're going to change
it to a non-static, this check will be added.
+ if (tolower(name[0]) == 'b') {
+ name++;
+ pin_nr = (simple_strtoul(name, (char **)&name, 10) <<
MXS_PAD_BANK_SHIFT) & MXS_PAD_BANK_MASK;
Does the name come from the user with the "gpio" command, right ? Then
the user can set any possible value here. Should we not check for the
maximum accepted value for MX28 GPIO before shifting ?
Correct; name the third gpio command line argument.
I don't see things going horribly wrong here. The ul value is shifted up and
masked with an identically shifted up mask of 0x1F. So any value above 31 will
be stripped of it's more significant bits and actual value will wrap around. In
other words, a value of 34 will end of a a value of 2, hence without being
caught as an invalid number. To me this is expected behavior on a 32-bit
system, but opinions may vary. I can check it here too, but then it'd be a
duplicate of what's already checked in my patch on gpio_request.
Perhaps it's best to move the pin validity check name_to_gpio. I've put int in
gpio_request because Blackfin put it there, but name_to_gpio really makes more
sense. Marek suggested a separate function checking pin validity, but I would
like to suggest to do this check in name_to_gpio, because this translation
produces multiple parameters that need to be checked, in this case a bank and a
pin number. And to perform a proper check, both of these paremeters must be
preserved in full and be parse to the validity check. On other platforms these
may be two different - incompatible - parameters, so I think validity checking
during the conversion is a good trade-off.
+ if (tolower(name[0]) == 'p') {
+ name++;
+ pin_nr |= (simple_strtoul(name, (char **)&name, 10) <<
MXS_PAD_PIN_SHIFT) & MXS_PAD_PIN_MASK;
Ditto
Ditto; The bank number is trimmed down to it's intended width of 3 bits. A
value of 5 and higher is translated, but later on rejected by the validity
check in gpio_request.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
