Am 15.03.2021 um 15:15 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@xxxxxxxxxx> writes:
>
> > Am 13.03.2021 um 13:30 hat Markus Armbruster geschrieben:
> >> Paolo Bonzini <pbonzini@xxxxxxxxxx> writes:
> >>
> >> > On 13/03/21 08:40, Markus Armbruster wrote:
> >> >>> + if (!user_creatable_add_from_str(optarg, &local_err)) {
> >> >>> + if (local_err) {
> >> >>> + error_report_err(local_err);
> >> >>> + exit(2);
> >> >>> + } else {
> >> >>> + /* Help was printed */
> >> >>> + exit(EXIT_SUCCESS);
> >> >>> + }
> >> >>> + }
> >> >>> + break;
> >> >>> }
> >> >>> - } break;
> >> >>> case OPTION_IMAGE_OPTS:
> >> >>> image_opts = true;
> >> >>> break;
> >> >> Why is this one different? The others all call
> >> >> user_creatable_process_cmdline().
> >> >>
> >> >>
> >> >
> >> > It's to exit with status code 2 instead of 1.
> >>
> >> I see. Worth a comment?
> >
> > There is a comment at the start of the function (which is just a few
> > lines above) that explains the exit codes:
> >
> > * Compares two images. Exit codes:
> > *
> > * 0 - Images are identical or the requested help was printed
> > * 1 - Images differ
> > * >1 - Error occurred
>
> I had in mind a comment that helps me over the "why is this not using
> user_creatable_process_cmdline()" hump. Like so:
>
> case OPTION_OBJECT:
> {
> /*
> * Can't use user_creatable_process_cmdline(), because
> * we need to exit(2) on error.
> */
> ... open-coded variation of
> user_creatable_process_cmdline() ...
> }
>
> Entirely up to you.
I see. This patch is already part of a pull request, but I wouldn't mind
a follow-up patch to add this comment if you want to send one.
Kevin
