Re: [PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux