From 6fecd78ca9dd86675b40938103b5ca83ad17b156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 10:15:45 +0100 Subject: udev/v4l_id: convert to run() and add error messages Also split out parse_argv(), use "ARG" instead of "". Make the syntax help a bit more precise. --- src/udev/v4l_id/v4l_id.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) (limited to 'src/udev') diff --git a/src/udev/v4l_id/v4l_id.c b/src/udev/v4l_id/v4l_id.c index 4f163c4350..98075db0ce 100644 --- a/src/udev/v4l_id/v4l_id.c +++ b/src/udev/v4l_id/v4l_id.c @@ -27,50 +27,64 @@ #include #include "fd-util.h" +#include "main-func.h" -int main(int argc, char *argv[]) { +static const char *arg_device = NULL; + +static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { { "help", no_argument, NULL, 'h' }, {} }; - _cleanup_close_ int fd = -EBADF; - char *device; - struct v4l2_capability v2cap; int c; while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) - switch (c) { case 'h': - printf("%s [-h,--help] \n\n" + printf("%s [OPTIONS...] DEVICE\n\n" "Video4Linux device identification.\n\n" - " -h Print this message\n", + " -h --help Show this help text\n", program_invocation_short_name); return 0; case '?': return -EINVAL; - default: assert_not_reached(); } - device = argv[optind]; - if (!device) - return 2; + if (!argv[optind]) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "DEVICE argument missing."); + + arg_device = argv[optind]; + return 1; +} - fd = open(device, O_RDONLY); +static int run(int argc, char *argv[]) { + _cleanup_close_ int fd = -EBADF; + struct v4l2_capability v2cap; + int r; + + r = parse_argv(argc, argv); + if (r <= 0) + return r; + + fd = open(arg_device, O_RDONLY); if (fd < 0) - return 3; + return log_error_errno(errno, "Failed to open %s: %m", arg_device); if (ioctl(fd, VIDIOC_QUERYCAP, &v2cap) == 0) { int capabilities; + printf("ID_V4L_VERSION=2\n"); printf("ID_V4L_PRODUCT=%s\n", v2cap.card); printf("ID_V4L_CAPABILITIES=:"); + if (v2cap.capabilities & V4L2_CAP_DEVICE_CAPS) capabilities = v2cap.device_caps; else capabilities = v2cap.capabilities; + if ((capabilities & V4L2_CAP_VIDEO_CAPTURE) > 0 || (capabilities & V4L2_CAP_VIDEO_CAPTURE_MPLANE) > 0) printf("capture:"); @@ -90,3 +104,5 @@ int main(int argc, char *argv[]) { return 0; } + +DEFINE_MAIN_FUNCTION(run); -- cgit v1.2.1 From 61da49cfd68a0eed918168735caea9134fe9b72e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 10:42:14 +0100 Subject: udev/ata_id: split out parse_argv() Beef up syntax and error messages a bit. --- src/udev/ata_id/ata_id.c | 81 ++++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 37 deletions(-) (limited to 'src/udev') diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index 6a631e515c..c1e0a6f3f3 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -31,6 +31,9 @@ #define COMMAND_TIMEOUT_MSEC (30 * 1000) +static bool arg_export = false; +static const char *arg_device = NULL; + static int disk_scsi_inquiry_command( int fd, void *buf, @@ -387,6 +390,39 @@ out: return ret; } +static int parse_argv(int argc, char *argv[]) { + static const struct option options[] = { + { "export", no_argument, NULL, 'x' }, + { "help", no_argument, NULL, 'h' }, + {} + }; + int c; + + while ((c = getopt_long(argc, argv, "xh", options, NULL)) >= 0) + switch (c) { + case 'x': + arg_export = true; + break; + case 'h': + printf("%s [OPTIONS...] DEVICE\n\n" + " -x --export Print values as environment keys\n" + " -h --help Show this help text\n", + program_invocation_short_name); + return 0; + case '?': + return -EINVAL; + default: + assert_not_reached(); + } + + if (!argv[optind]) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "DEVICE argument missing."); + + arg_device = argv[optind]; + return 1; +} + int main(int argc, char *argv[]) { struct hd_driveid id; union { @@ -397,51 +433,22 @@ int main(int argc, char *argv[]) { char model_enc[256]; char serial[21]; char revision[9]; - const char *node = NULL; - int export = 0; _cleanup_close_ int fd = -EBADF; uint16_t word; - int is_packet_device = 0; - static const struct option options[] = { - { "export", no_argument, NULL, 'x' }, - { "help", no_argument, NULL, 'h' }, - {} - }; + int is_packet_device = 0, r; log_set_target(LOG_TARGET_AUTO); udev_parse_config(); log_parse_environment(); log_open(); - for (;;) { - int option; - - option = getopt_long(argc, argv, "xh", options, NULL); - if (option == -1) - break; - - switch (option) { - case 'x': - export = 1; - break; - case 'h': - printf("Usage: %s [--export] [--help] \n" - " -x,--export print values as environment keys\n" - " -h,--help print this help text\n\n", - program_invocation_short_name); - return 0; - } - } - - node = argv[optind]; - if (!node) { - log_error("no node specified"); - return 1; - } + r = parse_argv(argc, argv); + if (r <= 0) + return r < 0 ? 1 : 0; - fd = open(node, O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); + fd = open(ASSERT_PTR(arg_device), O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); if (fd < 0) { - log_error("unable to open '%s'", node); + log_error_errno(errno, "Cannot open %s: %m", arg_device); return 1; } @@ -476,7 +483,7 @@ int main(int argc, char *argv[]) { } else { /* If this fails, then try HDIO_GET_IDENTITY */ if (ioctl(fd, HDIO_GET_IDENTITY, &id) != 0) { - log_debug_errno(errno, "HDIO_GET_IDENTITY failed for '%s': %m", node); + log_debug_errno(errno, "%s: HDIO_GET_IDENTITY failed: %m", arg_device); return 2; } } @@ -491,7 +498,7 @@ int main(int argc, char *argv[]) { udev_replace_whitespace((char *) id.fw_rev, revision, 8); udev_replace_chars(revision, NULL); - if (export) { + if (arg_export) { /* Set this to convey the disk speaks the ATA protocol */ printf("ID_ATA=1\n"); -- cgit v1.2.1 From c6f85c3c40627aca5963b534a132d64dbab3c118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 10:45:23 +0100 Subject: udev/ata_id: convert to run() --- src/udev/ata_id/ata_id.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'src/udev') diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index c1e0a6f3f3..72a35f0aa1 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -26,6 +26,7 @@ #include "device-nodes.h" #include "fd-util.h" #include "log.h" +#include "main-func.h" #include "memory-util.h" #include "udev-util.h" @@ -423,7 +424,7 @@ static int parse_argv(int argc, char *argv[]) { return 1; } -int main(int argc, char *argv[]) { +static int run(int argc, char *argv[]) { struct hd_driveid id; union { uint8_t byte[512]; @@ -444,13 +445,11 @@ int main(int argc, char *argv[]) { r = parse_argv(argc, argv); if (r <= 0) - return r < 0 ? 1 : 0; + return r; fd = open(ASSERT_PTR(arg_device), O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY); - if (fd < 0) { - log_error_errno(errno, "Cannot open %s: %m", arg_device); - return 1; - } + if (fd < 0) + return log_error_errno(errno, "Cannot open %s: %m", arg_device); if (disk_identify(fd, identify.byte, &is_packet_device) == 0) { /* @@ -482,10 +481,8 @@ int main(int argc, char *argv[]) { memcpy(&id, identify.byte, sizeof id); } else { /* If this fails, then try HDIO_GET_IDENTITY */ - if (ioctl(fd, HDIO_GET_IDENTITY, &id) != 0) { - log_debug_errno(errno, "%s: HDIO_GET_IDENTITY failed: %m", arg_device); - return 2; - } + if (ioctl(fd, HDIO_GET_IDENTITY, &id) != 0) + return log_debug_errno(errno, "%s: HDIO_GET_IDENTITY failed: %m", arg_device); } memcpy(model, id.model, 40); @@ -656,3 +653,5 @@ int main(int argc, char *argv[]) { return 0; } + +DEFINE_MAIN_FUNCTION(run); -- cgit v1.2.1 From 4dfe41e23475cfbabad25b2523624299485e223d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 10:56:17 +0100 Subject: udev/ata_id: drop unused output parameter --- src/udev/ata_id/ata_id.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) (limited to 'src/udev') diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index 72a35f0aa1..3053eb34f9 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -301,11 +301,10 @@ static void disk_identify_fixup_uint16 (uint8_t identify[512], unsigned offset_w * disk_identify: * @fd: File descriptor for the block device. * @out_identify: Return location for IDENTIFY data. - * @out_is_packet_device: Return location for whether returned data is from an IDENTIFY PACKET DEVICE. * * Sends the IDENTIFY DEVICE or IDENTIFY PACKET DEVICE command to the * device represented by @fd. If successful, then the result will be - * copied into @out_identify and @out_is_packet_device. + * copied into @out_identify. * * This routine is based on code from libatasmart, LGPL v2.1. * @@ -313,14 +312,11 @@ static void disk_identify_fixup_uint16 (uint8_t identify[512], unsigned offset_w * non-zero with errno set. */ static int disk_identify(int fd, - uint8_t out_identify[512], - int *out_is_packet_device) { + uint8_t out_identify[512]) { int ret; uint8_t inquiry_buf[36]; int peripheral_device_type; int all_nul_bytes; - int n; - int is_packet_device = 0; /* init results */ memzero(out_identify, 512); @@ -352,12 +348,10 @@ static int disk_identify(int fd, /* SPC-4, section 6.4.2: Standard INQUIRY data */ peripheral_device_type = inquiry_buf[0] & 0x1f; - if (peripheral_device_type == 0x05) - { - is_packet_device = 1; - ret = disk_identify_packet_device_command(fd, out_identify, 512); - goto check_nul_bytes; - } + if (peripheral_device_type == 0x05) { + ret = disk_identify_packet_device_command(fd, out_identify, 512); + goto check_nul_bytes; + } if (!IN_SET(peripheral_device_type, 0x00, 0x14)) { ret = -1; errno = EIO; @@ -372,12 +366,11 @@ static int disk_identify(int fd, check_nul_bytes: /* Check if IDENTIFY data is all NUL bytes - if so, bail */ all_nul_bytes = 1; - for (n = 0; n < 512; n++) { + for (size_t n = 0; n < 512; n++) if (out_identify[n] != '\0') { all_nul_bytes = 0; break; } - } if (all_nul_bytes) { ret = -1; @@ -386,8 +379,6 @@ static int disk_identify(int fd, } out: - if (out_is_packet_device) - *out_is_packet_device = is_packet_device; return ret; } @@ -430,13 +421,10 @@ static int run(int argc, char *argv[]) { uint8_t byte[512]; uint16_t wyde[256]; } identify; - char model[41]; - char model_enc[256]; - char serial[21]; - char revision[9]; + char model[41], model_enc[256], serial[21], revision[9]; _cleanup_close_ int fd = -EBADF; uint16_t word; - int is_packet_device = 0, r; + int r; log_set_target(LOG_TARGET_AUTO); udev_parse_config(); @@ -451,7 +439,7 @@ static int run(int argc, char *argv[]) { if (fd < 0) return log_error_errno(errno, "Cannot open %s: %m", arg_device); - if (disk_identify(fd, identify.byte, &is_packet_device) == 0) { + if (disk_identify(fd, identify.byte) == 0) { /* * fix up only the fields from the IDENTIFY data that we are going to * use and copy it into the hd_driveid struct for convenience -- cgit v1.2.1 From 66026fca95c892059b7dc4f33596f63741519eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 11:28:54 +0100 Subject: udev/ata_id: stop using errno, fix logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code was setting errno, which we don't do, and which is hard to get right… Rework the code to use the usual negative-errno convention and add some debug logging. disk_scsi_inquiry_command() was working partially by accident: the v3 fallback would enter the v4 check path, and pass it because the v4 data would still be initialized to 0. --- src/udev/ata_id/ata_id.c | 213 +++++++++++++++++++++-------------------------- 1 file changed, 95 insertions(+), 118 deletions(-) (limited to 'src/udev') diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index 3053eb34f9..ec472feada 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -59,45 +59,39 @@ static int disk_scsi_inquiry_command( .din_xferp = (uintptr_t) buf, .timeout = COMMAND_TIMEOUT_MSEC, }; - int ret; - ret = ioctl(fd, SG_IO, &io_v4); - if (ret != 0) { + if (ioctl(fd, SG_IO, &io_v4) != 0) { + if (errno != EINVAL) + return log_debug_errno(errno, "ioctl v4 failed: %m"); + /* could be that the driver doesn't do version 4, try version 3 */ - if (errno == EINVAL) { - struct sg_io_hdr io_hdr = { - .interface_id = 'S', - .cmdp = (unsigned char*) cdb, - .cmd_len = sizeof (cdb), - .dxferp = buf, - .dxfer_len = buf_len, - .sbp = sense, - .mx_sb_len = sizeof(sense), - .dxfer_direction = SG_DXFER_FROM_DEV, - .timeout = COMMAND_TIMEOUT_MSEC, - }; - - ret = ioctl(fd, SG_IO, &io_hdr); - if (ret != 0) - return ret; - - /* even if the ioctl succeeds, we need to check the return value */ - if (!(io_hdr.status == 0 && - io_hdr.host_status == 0 && - io_hdr.driver_status == 0)) { - errno = EIO; - return -1; - } - } else - return ret; - } + struct sg_io_hdr io_hdr = { + .interface_id = 'S', + .cmdp = (unsigned char*) cdb, + .cmd_len = sizeof (cdb), + .dxferp = buf, + .dxfer_len = buf_len, + .sbp = sense, + .mx_sb_len = sizeof(sense), + .dxfer_direction = SG_DXFER_FROM_DEV, + .timeout = COMMAND_TIMEOUT_MSEC, + }; + + if (ioctl(fd, SG_IO, &io_hdr) != 0) + return log_debug_errno(errno, "ioctl v3 failed: %m"); + + /* even if the ioctl succeeds, we need to check the return value */ + if (io_hdr.status != 0 || + io_hdr.host_status != 0 || + io_hdr.driver_status != 0) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "ioctl v3 failed"); - /* even if the ioctl succeeds, we need to check the return value */ - if (!(io_v4.device_status == 0 && - io_v4.transport_status == 0 && - io_v4.driver_status == 0)) { - errno = EIO; - return -1; + } else { + /* even if the ioctl succeeds, we need to check the return value */ + if (io_v4.device_status != 0 || + io_v4.transport_status != 0 || + io_v4.driver_status != 0) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "ioctl v4 failed"); } return 0; @@ -141,35 +135,30 @@ static int disk_identify_command( .din_xferp = (uintptr_t) buf, .timeout = COMMAND_TIMEOUT_MSEC, }; - int ret; - ret = ioctl(fd, SG_IO, &io_v4); - if (ret != 0) { - /* could be that the driver doesn't do version 4, try version 3 */ - if (errno == EINVAL) { - struct sg_io_hdr io_hdr = { - .interface_id = 'S', - .cmdp = (unsigned char*) cdb, - .cmd_len = sizeof (cdb), - .dxferp = buf, - .dxfer_len = buf_len, - .sbp = sense, - .mx_sb_len = sizeof (sense), - .dxfer_direction = SG_DXFER_FROM_DEV, - .timeout = COMMAND_TIMEOUT_MSEC, - }; - - ret = ioctl(fd, SG_IO, &io_hdr); - if (ret != 0) - return ret; - } else - return ret; - } + if (ioctl(fd, SG_IO, &io_v4) != 0) { + if (errno != EINVAL) + return log_debug_errno(errno, "ioctl v4 failed: %m"); - if (!((sense[0] & 0x7f) == 0x72 && desc[0] == 0x9 && desc[1] == 0x0c) && - !((sense[0] & 0x7f) == 0x70 && sense[12] == 0x00 && sense[13] == 0x1d)) { - errno = EIO; - return -1; + /* could be that the driver doesn't do version 4, try version 3 */ + struct sg_io_hdr io_hdr = { + .interface_id = 'S', + .cmdp = (unsigned char*) cdb, + .cmd_len = sizeof (cdb), + .dxferp = buf, + .dxfer_len = buf_len, + .sbp = sense, + .mx_sb_len = sizeof (sense), + .dxfer_direction = SG_DXFER_FROM_DEV, + .timeout = COMMAND_TIMEOUT_MSEC, + }; + + if (ioctl(fd, SG_IO, &io_hdr) != 0) + return log_debug_errno(errno, "ioctl v3 failed: %m"); + } else { + if (!((sense[0] & 0x7f) == 0x72 && desc[0] == 0x9 && desc[1] == 0x0c) && + !((sense[0] & 0x7f) == 0x70 && sense[12] == 0x00 && sense[13] == 0x1d)) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "ioctl v4 failed: %m"); } return 0; @@ -219,34 +208,29 @@ static int disk_identify_packet_device_command( .din_xferp = (uintptr_t) buf, .timeout = COMMAND_TIMEOUT_MSEC, }; - int ret; - ret = ioctl(fd, SG_IO, &io_v4); - if (ret != 0) { - /* could be that the driver doesn't do version 4, try version 3 */ - if (errno == EINVAL) { - struct sg_io_hdr io_hdr = { - .interface_id = 'S', - .cmdp = (unsigned char*) cdb, - .cmd_len = sizeof (cdb), - .dxferp = buf, - .dxfer_len = buf_len, - .sbp = sense, - .mx_sb_len = sizeof (sense), - .dxfer_direction = SG_DXFER_FROM_DEV, - .timeout = COMMAND_TIMEOUT_MSEC, - }; - - ret = ioctl(fd, SG_IO, &io_hdr); - if (ret != 0) - return ret; - } else - return ret; - } + if (ioctl(fd, SG_IO, &io_v4) != 0) { + if (errno != EINVAL) + return log_debug_errno(errno, "ioctl v4 failed: %m"); - if (!((sense[0] & 0x7f) == 0x72 && desc[0] == 0x9 && desc[1] == 0x0c)) { - errno = EIO; - return -1; + /* could be that the driver doesn't do version 4, try version 3 */ + struct sg_io_hdr io_hdr = { + .interface_id = 'S', + .cmdp = (unsigned char*) cdb, + .cmd_len = sizeof (cdb), + .dxferp = buf, + .dxfer_len = buf_len, + .sbp = sense, + .mx_sb_len = sizeof (sense), + .dxfer_direction = SG_DXFER_FROM_DEV, + .timeout = COMMAND_TIMEOUT_MSEC, + }; + + if (ioctl(fd, SG_IO, &io_hdr) != 0) + return log_debug_errno(errno, "ioctl v3 failed: %m"); + } else { + if ((sense[0] & 0x7f) != 0x72 || desc[0] != 0x9 || desc[1] != 0x0c) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "ioctl v4 failed: %m"); } return 0; @@ -313,10 +297,8 @@ static void disk_identify_fixup_uint16 (uint8_t identify[512], unsigned offset_w */ static int disk_identify(int fd, uint8_t out_identify[512]) { - int ret; uint8_t inquiry_buf[36]; - int peripheral_device_type; - int all_nul_bytes; + int peripheral_device_type, r; /* init results */ memzero(out_identify, 512); @@ -342,44 +324,39 @@ static int disk_identify(int fd, * the original bug-fix and see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=556635 * for the original bug-report.) */ - ret = disk_scsi_inquiry_command (fd, inquiry_buf, sizeof (inquiry_buf)); - if (ret != 0) - goto out; + r = disk_scsi_inquiry_command(fd, inquiry_buf, sizeof inquiry_buf); + if (r < 0) + return r; /* SPC-4, section 6.4.2: Standard INQUIRY data */ peripheral_device_type = inquiry_buf[0] & 0x1f; if (peripheral_device_type == 0x05) { - ret = disk_identify_packet_device_command(fd, out_identify, 512); - goto check_nul_bytes; - } - if (!IN_SET(peripheral_device_type, 0x00, 0x14)) { - ret = -1; - errno = EIO; - goto out; - } + r = disk_identify_packet_device_command(fd, out_identify, 512); + if (r < 0) + return r; - /* OK, now issue the IDENTIFY DEVICE command */ - ret = disk_identify_command(fd, out_identify, 512); - if (ret != 0) - goto out; + } else { + if (!IN_SET(peripheral_device_type, 0x00, 0x14)) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Unsupported device type."); + + /* OK, now issue the IDENTIFY DEVICE command */ + r = disk_identify_command(fd, out_identify, 512); + if (r < 0) + return r; + } - check_nul_bytes: /* Check if IDENTIFY data is all NUL bytes - if so, bail */ - all_nul_bytes = 1; + bool all_nul_bytes = true; for (size_t n = 0; n < 512; n++) if (out_identify[n] != '\0') { - all_nul_bytes = 0; + all_nul_bytes = false; break; } - if (all_nul_bytes) { - ret = -1; - errno = EIO; - goto out; - } + if (all_nul_bytes) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "IDENTIFY data is all zeroes."); -out: - return ret; + return 0; } static int parse_argv(int argc, char *argv[]) { @@ -439,7 +416,7 @@ static int run(int argc, char *argv[]) { if (fd < 0) return log_error_errno(errno, "Cannot open %s: %m", arg_device); - if (disk_identify(fd, identify.byte) == 0) { + if (disk_identify(fd, identify.byte) >= 0) { /* * fix up only the fields from the IDENTIFY data that we are going to * use and copy it into the hd_driveid struct for convenience -- cgit v1.2.1 From 4b0f96748411a1bef8bdc0bae6f9e9d316223028 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 11:39:44 +0100 Subject: udev/fido_id: implement --help --- src/udev/fido_id/fido_id.c | 47 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) (limited to 'src/udev') diff --git a/src/udev/fido_id/fido_id.c b/src/udev/fido_id/fido_id.c index f2fbc38003..11f5320d2b 100644 --- a/src/udev/fido_id/fido_id.c +++ b/src/udev/fido_id/fido_id.c @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -25,16 +26,43 @@ #include "string-util.h" #include "udev-util.h" +static const char *arg_device = NULL; + +static int parse_argv(int argc, char *argv[]) { + static const struct option options[] = { + { "help", no_argument, NULL, 'h' }, + {} + }; + int c; + + while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) + switch (c) { + case 'h': + printf("%s [OPTIONS...] SYSFS_PATH\n\n" + " -h --help Show this help text\n", + program_invocation_short_name); + return 0; + case '?': + return -EINVAL; + default: + assert_not_reached(); + } + + if (argc > 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Error: unexpected argument."); + + arg_device = argv[optind]; + return 1; +} + static int run(int argc, char **argv) { _cleanup_(sd_device_unrefp) struct sd_device *device = NULL; _cleanup_free_ char *desc_path = NULL; _cleanup_close_ int fd = -EBADF; - struct sd_device *hid_device; const char *sys_path; uint8_t desc[HID_MAX_DESCRIPTOR_SIZE]; ssize_t desc_len; - int r; log_set_target(LOG_TARGET_AUTO); @@ -42,17 +70,18 @@ static int run(int argc, char **argv) { log_parse_environment(); log_open(); - if (argc > 2) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Usage: %s [SYSFS_PATH]", program_invocation_short_name); + r = parse_argv(argc, argv); + if (r <= 0) + return r; - if (argc == 1) { - r = device_new_from_strv(&device, environ); + if (arg_device) { + r = sd_device_new_from_syspath(&device, arg_device); if (r < 0) - return log_error_errno(r, "Failed to get current device from environment: %m"); + return log_error_errno(r, "Failed to get device from syspath %s: %m", arg_device); } else { - r = sd_device_new_from_syspath(&device, argv[1]); + r = device_new_from_strv(&device, environ); if (r < 0) - return log_error_errno(r, "Failed to get device from syspath: %m"); + return log_error_errno(r, "Failed to get current device from environment: %m"); } r = sd_device_get_parent(device, &hid_device); -- cgit v1.2.1 From 8b89b9d7c324953ac8c09e2f04f47246e39aa6cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 11:46:01 +0100 Subject: udev/mtd_probe: implement --help --- src/udev/mtd_probe/mtd_probe.c | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) (limited to 'src/udev') diff --git a/src/udev/mtd_probe/mtd_probe.c b/src/udev/mtd_probe/mtd_probe.c index a7210a05e3..fd6267feb5 100644 --- a/src/udev/mtd_probe/mtd_probe.c +++ b/src/udev/mtd_probe/mtd_probe.c @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -32,14 +33,43 @@ #include "fd-util.h" #include "mtd_probe.h" +static const char *arg_device = NULL; + +static int parse_argv(int argc, char *argv[]) { + static const struct option options[] = { + { "help", no_argument, NULL, 'h' }, + {} + }; + int c; + + while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) + switch (c) { + case 'h': + printf("%s /dev/mtd[n]\n\n" + " -h --help Show this help text\n", + program_invocation_short_name); + return 0; + case '?': + return -EINVAL; + default: + assert_not_reached(); + } + + if (argc > 2) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Error: unexpected argument."); + + arg_device = argv[optind]; + return 1; +} + int main(int argc, char** argv) { _cleanup_close_ int mtd_fd = -EBADF; mtd_info_t mtd_info; + int r; - if (argc != 2) { - printf("usage: mtd_probe /dev/mtd[n]\n"); - return EXIT_FAILURE; - } + r = parse_argv(argc, argv); + if (r <= 0) + return r < 0 ? EXIT_FAILURE : 0; mtd_fd = open(argv[1], O_RDONLY|O_CLOEXEC|O_NOCTTY); if (mtd_fd < 0) { -- cgit v1.2.1 From 32c2d046c67b9ecbfb24e9492476159259c346aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 11:49:48 +0100 Subject: udev/mtd_probe: convert to run() --- src/udev/mtd_probe/mtd_probe.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) (limited to 'src/udev') diff --git a/src/udev/mtd_probe/mtd_probe.c b/src/udev/mtd_probe/mtd_probe.c index fd6267feb5..513491ed6c 100644 --- a/src/udev/mtd_probe/mtd_probe.c +++ b/src/udev/mtd_probe/mtd_probe.c @@ -31,6 +31,7 @@ #include "alloc-util.h" #include "fd-util.h" +#include "main-func.h" #include "mtd_probe.h" static const char *arg_device = NULL; @@ -62,28 +63,23 @@ static int parse_argv(int argc, char *argv[]) { return 1; } -int main(int argc, char** argv) { +static int run(int argc, char** argv) { _cleanup_close_ int mtd_fd = -EBADF; mtd_info_t mtd_info; int r; r = parse_argv(argc, argv); if (r <= 0) - return r < 0 ? EXIT_FAILURE : 0; + return r; mtd_fd = open(argv[1], O_RDONLY|O_CLOEXEC|O_NOCTTY); - if (mtd_fd < 0) { - log_error_errno(errno, "Failed to open: %m"); - return EXIT_FAILURE; - } + if (mtd_fd < 0) + return log_error_errno(errno, "Failed to open: %m"); - if (ioctl(mtd_fd, MEMGETINFO, &mtd_info) < 0) { - log_error_errno(errno, "Failed to issue MEMGETINFO ioctl: %m"); - return EXIT_FAILURE; - } + if (ioctl(mtd_fd, MEMGETINFO, &mtd_info) < 0) + return log_error_errno(errno, "MEMGETINFO ioctl failed: %m"); - if (probe_smart_media(mtd_fd, &mtd_info) < 0) - return EXIT_FAILURE; - - return EXIT_SUCCESS; + return probe_smart_media(mtd_fd, &mtd_info); } + +DEFINE_MAIN_FUNCTION(run); -- cgit v1.2.1 From bd36d0281ab41bfea932931e913809ee1e6195ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 11:59:43 +0100 Subject: udev/cdrom_id: do not abort on unknown options Copy-pasta is good, but you have to pick a reliable source. --- src/udev/cdrom_id/cdrom_id.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/udev') diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index ea420a9617..3d58100f3e 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -938,6 +938,8 @@ static int parse_argv(int argc, char *argv[]) { break; case 'h': return help(); + case '?': + return -EINVAL; default: assert_not_reached(); } -- cgit v1.2.1 From 5356761da67aa5c11f784c3ba9ac8d8f6e55048b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 18:12:04 +0100 Subject: udev: implement --version in all builtins Those are separate binaries, and occasionally people will get a misplaced binary that doesn't match the rest of the installed system and be confused, so it good to be able to check the version. It is also nice to have the same interface in all binaries. Note that we usually use a separate 'enum ARG_VERSION = 0x100' for an option without a short name. We can use a less verbose approach of simply taking any unused letter, which works just as well and even the compiler would warn us if we tried to use the letter in another place. This way we avoid a few lines of boilerplate. The help texts are adjusted to have an empty line between the synopsis and option list, and no empty lines after the option list. --- src/udev/ata_id/ata_id.c | 11 ++++++++--- src/udev/cdrom_id/cdrom_id.c | 18 +++++++++++------- src/udev/dmi_memory_id/dmi_memory_id.c | 11 ++++++++--- src/udev/fido_id/fido_id.c | 9 +++++++-- src/udev/mtd_probe/mtd_probe.c | 9 +++++++-- src/udev/v4l_id/v4l_id.c | 9 +++++++-- 6 files changed, 48 insertions(+), 19 deletions(-) (limited to 'src/udev') diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index ec472feada..7760d248cf 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -23,6 +23,7 @@ #include #include +#include "build.h" #include "device-nodes.h" #include "fd-util.h" #include "log.h" @@ -361,8 +362,9 @@ static int disk_identify(int fd, static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { - { "export", no_argument, NULL, 'x' }, - { "help", no_argument, NULL, 'h' }, + { "export", no_argument, NULL, 'x' }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'v' }, {} }; int c; @@ -375,9 +377,12 @@ static int parse_argv(int argc, char *argv[]) { case 'h': printf("%s [OPTIONS...] DEVICE\n\n" " -x --export Print values as environment keys\n" - " -h --help Show this help text\n", + " -h --help Show this help text\n" + " --version Show package version\n", program_invocation_short_name); return 0; + case 'v': + return version(); case '?': return -EINVAL; default: diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index 3d58100f3e..5b5fbd0109 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -10,6 +10,7 @@ #include #include +#include "build.h" #include "fd-util.h" #include "main-func.h" #include "memory-util.h" @@ -897,13 +898,13 @@ static void print_properties(const Context *c) { } static int help(void) { - printf("Usage: %s [options] \n" - " -l --lock-media lock the media (to enable eject request events)\n" - " -u --unlock-media unlock the media\n" - " -e --eject-media eject the media\n" - " -d --debug print debug messages to stderr\n" - " -h --help print this help text\n" - "\n", + printf("%s [OPTIONS...] DEVICE\n\n" + " -l --lock-media Lock the media (to enable eject request events)\n" + " -u --unlock-media Unlock the media\n" + " -e --eject-media Eject the media\n" + " -d --debug Print debug messages to stderr\n" + " -h --help Show this help text\n" + " --version Show package version\n", program_invocation_short_name); return 0; @@ -916,6 +917,7 @@ static int parse_argv(int argc, char *argv[]) { { "eject-media", no_argument, NULL, 'e' }, { "debug", no_argument, NULL, 'd' }, { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'v' }, {} }; int c; @@ -938,6 +940,8 @@ static int parse_argv(int argc, char *argv[]) { break; case 'h': return help(); + case 'v': + return version(); case '?': return -EINVAL; default: diff --git a/src/udev/dmi_memory_id/dmi_memory_id.c b/src/udev/dmi_memory_id/dmi_memory_id.c index 1345289219..dd46113137 100644 --- a/src/udev/dmi_memory_id/dmi_memory_id.c +++ b/src/udev/dmi_memory_id/dmi_memory_id.c @@ -45,6 +45,7 @@ #include #include "alloc-util.h" +#include "build.h" #include "fileio.h" #include "main-func.h" #include "string-util.h" @@ -638,9 +639,10 @@ static int legacy_decode(const uint8_t *buf, const char *devmem, bool no_file_of } static int help(void) { - printf("Usage: %s [options]\n" - " -F,--from-dump FILE read DMI information from a binary file\n" - " -h,--help print this help text\n\n", + printf("%s [OPTIONS...]\n\n" + " -F --from-dump FILE Read DMI information from a binary file\n" + " -h --help Show this help text\n" + " --version Show package version\n", program_invocation_short_name); return 0; } @@ -650,6 +652,7 @@ static int parse_argv(int argc, char * const *argv) { { "from-dump", required_argument, NULL, 'F' }, { "version", no_argument, NULL, 'V' }, { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'v' }, {} }; int c; @@ -666,6 +669,8 @@ static int parse_argv(int argc, char * const *argv) { return help(); case '?': return -EINVAL; + case 'v': + return version(); default: assert_not_reached(); } diff --git a/src/udev/fido_id/fido_id.c b/src/udev/fido_id/fido_id.c index 11f5320d2b..e01f37d04c 100644 --- a/src/udev/fido_id/fido_id.c +++ b/src/udev/fido_id/fido_id.c @@ -15,6 +15,7 @@ #include #include +#include "build.h" #include "device-private.h" #include "device-util.h" #include "fd-util.h" @@ -30,7 +31,8 @@ static const char *arg_device = NULL; static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'v' }, {} }; int c; @@ -39,9 +41,12 @@ static int parse_argv(int argc, char *argv[]) { switch (c) { case 'h': printf("%s [OPTIONS...] SYSFS_PATH\n\n" - " -h --help Show this help text\n", + " -h --help Show this help text\n" + " --version Show package version\n", program_invocation_short_name); return 0; + case 'v': + return version(); case '?': return -EINVAL; default: diff --git a/src/udev/mtd_probe/mtd_probe.c b/src/udev/mtd_probe/mtd_probe.c index 513491ed6c..1035320490 100644 --- a/src/udev/mtd_probe/mtd_probe.c +++ b/src/udev/mtd_probe/mtd_probe.c @@ -30,6 +30,7 @@ #include #include "alloc-util.h" +#include "build.h" #include "fd-util.h" #include "main-func.h" #include "mtd_probe.h" @@ -38,7 +39,8 @@ static const char *arg_device = NULL; static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'v' }, {} }; int c; @@ -47,9 +49,12 @@ static int parse_argv(int argc, char *argv[]) { switch (c) { case 'h': printf("%s /dev/mtd[n]\n\n" - " -h --help Show this help text\n", + " -h --help Show this help text\n" + " --version Show package version\n", program_invocation_short_name); return 0; + case 'v': + return version(); case '?': return -EINVAL; default: diff --git a/src/udev/v4l_id/v4l_id.c b/src/udev/v4l_id/v4l_id.c index 98075db0ce..1d176a387e 100644 --- a/src/udev/v4l_id/v4l_id.c +++ b/src/udev/v4l_id/v4l_id.c @@ -26,6 +26,7 @@ #include #include +#include "build.h" #include "fd-util.h" #include "main-func.h" @@ -33,7 +34,8 @@ static const char *arg_device = NULL; static int parse_argv(int argc, char *argv[]) { static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'v' }, {} }; int c; @@ -43,9 +45,12 @@ static int parse_argv(int argc, char *argv[]) { case 'h': printf("%s [OPTIONS...] DEVICE\n\n" "Video4Linux device identification.\n\n" - " -h --help Show this help text\n", + " -h --help Show this help text\n" + " --version Show package version\n", program_invocation_short_name); return 0; + case 'v': + return version(); case '?': return -EINVAL; default: -- cgit v1.2.1 From 8a4c11e0e2ef39dc3e94b655e7bfba3d68e60104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 13 Mar 2023 12:13:51 +0100 Subject: meson: add udev builtins to dist-check They now pass, and we might as well test them to avoid unexpected regressions. --- src/udev/meson.build | 1 + 1 file changed, 1 insertion(+) (limited to 'src/udev') diff --git a/src/udev/meson.build b/src/udev/meson.build index af7dea0dce..5b44dd8d7d 100644 --- a/src/udev/meson.build +++ b/src/udev/meson.build @@ -143,6 +143,7 @@ foreach prog : udev_progs install_dir : udevlibexecdir) udev_prog_paths += {name : exe} + public_programs += exe endforeach if install_sysconfdir_samples -- cgit v1.2.1 From 42fff80cb8fcd0df2d22d9875319a8e6c52efcae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 16 Mar 2023 16:40:00 +0100 Subject: udev/v4l_id: use O_CLOEXEC|O_NOCTTY This is the usual set of flags. O_CLOEXEC doesn't matter because we don't spawn anything, but O_NOCTTY could possibly make a difference if the helper is called on a wrong device type. --- src/udev/v4l_id/v4l_id.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/udev') diff --git a/src/udev/v4l_id/v4l_id.c b/src/udev/v4l_id/v4l_id.c index 1d176a387e..30527e9556 100644 --- a/src/udev/v4l_id/v4l_id.c +++ b/src/udev/v4l_id/v4l_id.c @@ -74,7 +74,7 @@ static int run(int argc, char *argv[]) { if (r <= 0) return r; - fd = open(arg_device, O_RDONLY); + fd = open(arg_device, O_RDONLY|O_CLOEXEC|O_NOCTTY); if (fd < 0) return log_error_errno(errno, "Failed to open %s: %m", arg_device); -- cgit v1.2.1 From fe0637079fa943ed4255745c648f94ae33a635f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 16 Mar 2023 16:42:32 +0100 Subject: udev/ata_id: use unliagned helpers The array is a union, aligned as uint16_t, so we were accessing fields at offsets of two, so we didn't do actually do unaligned access. But let's make this explicit. --- src/udev/ata_id/ata_id.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src/udev') diff --git a/src/udev/ata_id/ata_id.c b/src/udev/ata_id/ata_id.c index 7760d248cf..0b1f0b7157 100644 --- a/src/udev/ata_id/ata_id.c +++ b/src/udev/ata_id/ata_id.c @@ -30,6 +30,7 @@ #include "main-func.h" #include "memory-util.h" #include "udev-util.h" +#include "unaligned.h" #define COMMAND_TIMEOUT_MSEC (30 * 1000) @@ -271,15 +272,15 @@ static void disk_identify_fixup_string( uint8_t identify[512], unsigned offset_words, size_t len) { + assert(offset_words < 512/2); disk_identify_get_string(identify, offset_words, (char *) identify + offset_words * 2, len); } -static void disk_identify_fixup_uint16 (uint8_t identify[512], unsigned offset_words) { - uint16_t *p; - - p = (uint16_t *) identify; - p[offset_words] = le16toh (p[offset_words]); +static void disk_identify_fixup_uint16(uint8_t identify[512], unsigned offset_words) { + assert(offset_words < 512/2); + unaligned_write_ne16(identify + offset_words * 2, + unaligned_read_le16(identify + offset_words * 2)); } /** -- cgit v1.2.1