[Libguestfs] [PATCH nbdkit 1/2] file: Add an internal "mode"

[Laszlo Ersek]

On 08/18/22 11:32, Richard W.M. Jones wrote:
> On Thu, Aug 18, 2022 at 10:27:11AM +0200, Laszlo Ersek wrote:
>> On 08/17/22 17:38, Richard W.M. Jones wrote:
>>> Previously we relied on the implicit assumption filename xor directory,
>>> representing two modes.  Make this explicit with an internal mode
>>> variable.
>>>
>>> This is just refactoring and should not change the functionality.
>>> However we're now more strict about duplicate file= or dir= parameters
>>> appearing on the command line.  Previously only the last one had an
>>> effect and the others were silently ignored.  Now we give an error in
>>> this case.  eg this worked before but now gives an error:
>>>
>>>   $ ./nbdkit file /var/tmp /var/tmp/fedora-36.img
>>>   nbdkit: error: file|dir parameter can only appear once on the command line
>>> ---
>>>  plugins/file/file.c | 124 +++++++++++++++++++++++++++-----------------
>>>  1 file changed, 76 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/plugins/file/file.c b/plugins/file/file.c
>>> index aefca9ee2..f673ec132 100644
>>> --- a/plugins/file/file.c
>>> +++ b/plugins/file/file.c
>>> @@ -70,6 +70,7 @@
>>>  #include "isaligned.h"
>>>  #include "fdatasync.h"
>>>  
>>> +static enum { mode_none, mode_filename, mode_directory } mode = mode_none;
>>>  static char *filename = NULL;
>>>  static char *directory = NULL;
>>>  
>>> @@ -180,14 +181,23 @@ file_config (const char *key, const char *value)
>>>     * existence checks to the last possible moment.
>>>     */
>>>    if (strcmp (key, "file") == 0) {
>>> -    free (filename);
>>> +    if (mode != mode_none) {
>>> +    wrong_mode:
>>
>> *shudder*
>>
>> please move error handling sections to the end of the function. It's OK
>> to jump forward in exceptional cases, but crossing jump lines (i.e.
>> improper balancing / nesting) and backward jumps for error handling are
>> terrible.
> 
> I don't think I really have a problem with it, but I moved it
> to the end of the function.

Thank you.
Laszlo