Here's the struct as specified in the kernel: /* From /drivers/scsi/scsi_sysfs.c in the kernel: * sdev_rd_attr (type, "%d\n"); * sdev_rd_attr (vendor, "%.8s\n"); * sdev_rd_attr (model, "%.16s\n"); * sdev_rd_attr (rev, "%.4s\n"); */ As you can see, type is always printed with %d which should always be a valid signed integer.
Could you please give an example of a type for which atoi is not sufficient? The usage of errno and bb_strtoi increases the size of the binary, so unless it's required I don't think it should be used. Martin On Fri, 3 Jul 2020 at 14:45, Markus Gothe <nietzs...@lysator.liu.se> wrote: > As the original author of the applet I don't want to see atoi() calls in > the code. You know what atoi() returns when it fails? Same as atoi("0"). > > Please use bb_stroi() for signed integers. > > BR, > Markus > > Sent from my BlackBerry - the most secure mobile device > > > Original Message > > > From: martin.lewis....@gmail.com > Sent: June 11, 2020 15:45 > To: busybox@busybox.net > Subject: [PATCH 3/4] lsscsi: code shrink and refactor > > > Remove the use of strou because scsi device types are signed (as seen in > the kernel's code). > Improve the representation of SCSI_DEVICE_LIST. > > function old new delta > .rodata 159915 159920 +5 > lsscsi_main 364 349 -15 > > ------------------------------------------------------------------------------ > (add/remove: 0/0 grow/shrink: 1/1 up/down: 5/-15) Total: -10 > bytes > text data bss dec hex filename > 981332 16915 1872 1000119 f42b7 busybox_old > 981322 16915 1872 1000109 f42ad busybox_unstripped > > Signed-off-by: Martin Lewis <martin.lewis....@gmail.com> > --- > miscutils/lsscsi.c | 50 ++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 34 insertions(+), 16 deletions(-) > > diff --git a/miscutils/lsscsi.c b/miscutils/lsscsi.c > index f737d33d9..e29bedd77 100644 > --- a/miscutils/lsscsi.c > +++ b/miscutils/lsscsi.c > @@ -25,6 +25,28 @@ > > #include "libbb.h" > > +#define SCSI_DEVICE_TYPE(type, name) type name "\0" > +#define SCSI_DEVICE_TYPE_LIST \ > + SCSI_DEVICE_TYPE("\x00", "disk") \ > + SCSI_DEVICE_TYPE("\x01", "tape") \ > + SCSI_DEVICE_TYPE("\x02", "printer") \ > + SCSI_DEVICE_TYPE("\x03", "process") \ > + SCSI_DEVICE_TYPE("\x04", "worm") \ > + SCSI_DEVICE_TYPE("\x06", "scanner") \ > + SCSI_DEVICE_TYPE("\x07", "optical") \ > + SCSI_DEVICE_TYPE("\x08", "mediumx") \ > + SCSI_DEVICE_TYPE("\x09", "comms") \ > + SCSI_DEVICE_TYPE("\x0c", "storage") \ > + SCSI_DEVICE_TYPE("\x0d", "enclosu") \ > + SCSI_DEVICE_TYPE("\x0e", "sim dsk") \ > + SCSI_DEVICE_TYPE("\x0f", "opti rd") \ > + SCSI_DEVICE_TYPE("\x10", "bridge") \ > + SCSI_DEVICE_TYPE("\x11", "osd") \ > + SCSI_DEVICE_TYPE("\x12", "adi") \ > + SCSI_DEVICE_TYPE("\x1e", "wlun") \ > + SCSI_DEVICE_TYPE("\x1f", "no dev") > + > + > static const char scsi_dir[] ALIGN1 = "/sys/bus/scsi/devices"; > > static char *get_line(const char *filename, char *buf, unsigned *bufsize_p) > @@ -59,6 +81,13 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv > UNUSED_PARAM) > > dir = xopendir("."); > while ((de = readdir(dir)) != NULL) { > + /* > + * From /drivers/scsi/scsi_sysfs.c in the kernel: > + * sdev_rd_attr (type, "%d\n"); > + * sdev_rd_attr (vendor, "%.8s\n"); > + * sdev_rd_attr (model, "%.16s\n"); > + * sdev_rd_attr (rev, "%.4s\n"); > + */ > char buf[256]; > char *ptr; > unsigned bufsize; > @@ -67,7 +96,7 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv > UNUSED_PARAM) > const char *type_name; > const char *model; > const char *rev; > - unsigned type; > + int type; > > if (!isdigit(de->d_name[0])) > continue; > @@ -88,23 +117,12 @@ int lsscsi_main(int argc UNUSED_PARAM, char **argv > UNUSED_PARAM) > > printf("[%s]\t", de->d_name); > > -#define scsi_device_types \ > - "disk\0" "tape\0" "printer\0" "process\0" \ > - "worm\0" "\0" "scanner\0" "optical\0" \ > - "mediumx\0" "comms\0" "\0" "\0" \ > - "storage\0" "enclosu\0" "sim dsk\0" "opti rd\0" \ > - "bridge\0" "osd\0" "adi\0" "\0" \ > - "\0" "\0" "\0" "\0" \ > - "\0" "\0" "\0" "\0" \ > - "\0" "\0" "wlun\0" "no dev" > - type = bb_strtou(type_str, NULL, 10); > - if (errno > - || type >= 0x20 > - || (type_name = nth_string(scsi_device_types, type))[0] == '\0' > - ) { > + type = atoi(type_str); /* type_str is "%d\n" so atoi is sufficient */ > + if (type >= 0x20 || > + (type_name = memchr(SCSI_DEVICE_TYPE_LIST, type, > sizeof(SCSI_DEVICE_TYPE_LIST))) == NULL) { > printf("(%s)\t", type_str); > } else { > - printf("%s\t", type_name); > + printf("%s\t", type_name + 1); > } > > printf("%s\t""%s\t""%s\n", > -- > 2.11.0 > > _______________________________________________ > busybox mailing list > busybox@busybox.net > http://lists.busybox.net/mailman/listinfo/busybox >
_______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox