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

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.