
Dear Pavel Herrmann,
[...]
move this out.
besides, I think it'd be much systematic to just scream at user to call "sata rescan" and bail out instead of doing it for him.
i dont actually need a sata rescan, i just need to make sure i have dynamically allocated names, so i can safely call free() later, otherwise this segfaults when called before sata scan
See my previous email
- /* make sure we have valid filenames */
- if (!zeroed) {
init_sata(0);
zeroed = 1;
- }
- switch (argc) {
- case 0:
- case 1:
return CMD_RET_USAGE;
- case 2:
dev = simple_strtoul(argv[1], NULL, 10);
Ok, so if I run this command and ask for device 0xb00bf33d ... will this survive? Hint: it won't, rangecheck missing.
hint - scan_sata does the range check in this codepath
saw it, see below. Duplication of code is not good and is error prone.
return scan_sata(dev);
- case 3:
if (!strncmp(argv[1], "inf", 3)) {
dev = simple_strtoul(argv[2], NULL, 10);
Same here
see above
return scan_sata(dev);
}
return CMD_RET_USAGE;
- case 4:
if (!strncmp(argv[1], "load", 4)) {
dev = simple_strtoul(argv[2], NULL, 10);
if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) {
And here you have it ?
Uh oh, I see, sata_scan() does it for you ... I'd say, abstract it out into lightweight static inline function.
printf("File index %d is out of range.\n", dev);
return -EINVAL;
}
free(filenames[dev]);
filenames[dev] = strdup(argv[3]);
init_sata(dev);
return scan_sata(dev);
}
return CMD_RET_USAGE;
- }
- return CMD_RET_USAGE;
+}
+U_BOOT_CMD(
- sata_loop, 4, 1, do_loop,
- "SATA loopback",
- "[info] devnum - show info about loop devnum\n"
Make this "info" part mandatory. Than you can cut the whole argc loop into simple "if argc != 2 ; then fail" . And do simple checking for the first letter of the argument being either i or d .
wont help, still need argc 3 or 4
Makes is simpler, you can bail out if it's not 3 or 4
- "sata_loop load devnum file - load file from host FS into loop
devnum"
sata_loop is redundant above.
really? how do you figure?
Run "help sata_loop" and see the result ...
Pavel Herrmann