In cmd_i2c the following snippet of code appears 6 times so it would
be a good candidate to factor out into a static helper function:
/*
* I2C chip address
*/
chip = simple_strtoul(argv[1], NULL, 16);
/*
* I2C data address within the chip. This can be 1 or
* 2 bytes long. Some day it might be 3 bytes long :-).
*/
devaddr = simple_strtoul(argv[2], NULL, 16);
alen = 1;
for (j = 0; j < 8; j++) {
if (argv[2][j] == '.') {
alen = argv[2][j+1] - '0';
if (alen > 3) {
cmd_usage(cmdtp);
return 1;
}
break;
} else if (argv[2][j] == '\0')
break;
}
Now before I actually start doing that a few questions.
Should I factor out all lines and create a helper function with three
pointer arguments to contain the return values chip, devaddr and alen
(apart from argv and cmdptp which are input parameters).
Or would it be better to only factor out only the alen code and have a
function like:
int get_alen(char *str, cmd_tbl_t *cmdtp)
{
int alen, j;
alen = 1;
for (j = 0; j < 8; j++) {
if str[j] == '.') {
alen = str[j+1] - '0';
if (alen > 3) {
cmd_usage(cmdtp);
return -1;
}
break;
} else if (str[j] == '\0')
break;
return alen;
}
and call it by something like
alen = get_alen(argv[2]);
if (alen < 0) return alen; /* or return -1 */
Your opinion on the preferred solution is appreciated (and ofc keeping
the current code is also an option).
Frans
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
