On Sat, 16.05.15 10:18, Per Bergqvist (p...@bst.lu) wrote: > Lennart, > > Thank you for all the comments. > > I have changed everything except the 'No space between function name and > opening “(“‘. > Cannot find anything about that in CODING_STYLE or evidence in other > sources.
Hmm? This is from CODING_STYLE: - Do not write "foo ()", write "foo()". > + > +static int nvme_identify(struct udev *udev, int fd, void *buf, __u32 > buf_len) { > + struct nvme_admin_cmd command = { > + .opcode = nvme_admin_identify, > + .addr = (unsigned long)buf, > + .data_len = buf_len, > + .cdw10 = 1 }; Hmm, why __u32? First of all, try to use userspace types, i.e. uint32_t. But secondly, shouldn't this be size_t? > +int main(int argc, char *argv[]) { > + _cleanup_udev_unref_ struct udev *udev = NULL; > + > + struct nvme_id_ctrl nvme_id_ctrl = { .vid = 0 }; initializer appears unnecessary, as the compiler initializes all fields to 0 anyway if they are unspecified. i.e. just write "= { };" here... > + node = argv[optind]; > + if (node == NULL) { > + log_error("no node specified"); > + return EXIT_FAILURE; > + } > + > + fd = open(node, O_RDONLY|O_NONBLOCK|O_CLOEXEC); > + if (fd < 0) { > + log_error_errno(errno, "Unable to open '%s': %m", node); > + return -errno; needs to be "return EXIT_FAILURE". After all this is the main() function, where errors are not errno-style, but EXIT_FAILURE, EXIT_SUCCESS or something else betwee 0..255... > + } > + > + if (nvme_identify(udev, fd, &nvme_id_ctrl, sizeof(struct > nvme_id_ctrl)) == 0) { > + memcpy (model, nvme_id_ctrl.mn, > sizeof(nvme_id_ctrl.mn)); Please remove the space... > + if (serial[0] != '\0') { > + printf("ID_SERIAL=%s_%s\n", model, serial); > + printf("ID_SERIAL_SHORT=%s\n", serial); > + } else { > + printf("ID_SERIAL=%s\n", model); > + } Please no {} for single-line if blocks (or else blocks...) To me this looks fine otherwise, but I figure Kay has to decide if this goes in or not. He might want this as built-in rather than as external tool though... Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel