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

Reply via email to