4 comments:
Patch Set #2, Line 74: static void cli_classic_abort_usage(const char *msg)
Minor: Is there some sort of "noreturn" attribute for functions like this one? I guess it could be used here.
Patch Set #2, Line 82: int operation_specified
Does this work? AFAIK, this parameter gets passed by value, so incrementing it inside the function will not result in a change outside of it.
I guess you would want to pass a reference (pointer) instead.
Patch Set #2, Line 82: cli_classic_single_operation
A function name without a verb? Since functions do things, I would appreciate if the name had a verb somewhere, e.g.: check, test.
Maybe something like "test single operation or abort" would reflect what this function does better.
if (layoutfile) {
cli_classic_abort_usage("Error: --layout specified more than once. Aborting.\n");
}
if (ifd) {
cli_classic_abort_usage("Error: --layout and --ifd both specified. Aborting.\n");
}
if (fmap) {
cli_classic_abort_usage("Error: --layout and --fmap-file both specified. Aborting.\n");
}
I shall point out that this is quite redundant as well, so that you can't unsee it :)
To view, visit change 35611. To unsubscribe, or for help writing mail filters, visit settings.