On Tue, Apr 16, 2013 at 10:49 AM, Thomas Bächler <thomas@xxxxxxxxxxxxx> wrote:
> Am 16.04.2013 15:12, schrieb Tom Gundersen:
>> +static void write_human(FILE *out, char module[], char devname[], char type, unsigned int maj, unsigned int min)
>
> [...]
>
>> +static void write_tmpfile(FILE *out, char devname[], char type, unsigned int maj, unsigned int min)
>
> [...]
>
>> +static int do_static_nodes(int argc, char *argv[])
>> +{
>> + struct utsname kernel;
>> + char modules[PATH_MAX];
>> + FILE *in = NULL, *out = stdout;
>> + bool human_readable = 1;
>
> This code emphasizes that there is actually only one format available
> and needs to be changed again when another one is added. Why not
>
> void (*write_output)((FILE *, char[], char[], char, unsigned int,
> unsigned int) = write_human;
>
> ? Then ...
>
>> + case 'f':
>> + if (!streq(optarg, "tmpfiles")) {
>> + fprintf(stderr, "Unknown format: '%s'.\n", argv[1]);
>> + help();
>> + ret = EXIT_FAILURE;
>> + goto finish;
>> + }
>> + human_readable = 0;
>> + break;
>
> case 'f':
> if (streq(optarg, "tmpfiles")) {
> write_output = write_tmpfiles;
> }
> else {
> fprintf(stderr, "Unknown format: '%s'.\n", argv[1]);
> [...]
> }
> break;
>
> And in the end:
>
>> + if (human_readable)
>> + write_human(out, module, devname, type, maj, min);
>> + else
>> + write_tmpfile(out, devname, type, maj, min);
>
> write_output(out, module, devname, type, maj, min);
>
> Maybe even add an array with output name and function pointer pairs, so
> that we could get a list of available formats using --format=?. For
> consistency, --format=human should also work. Just seems nicer to me, in
> case someone actually plans to extend this later.
>
Agree. Otherwise looks good.
Kay, what's your opinion regarding this command? Is this sufficient to
drop CAP_MKNOD from udev?
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
