From 41053e9dcd153744b61bd95b822aa0af81a14d6e Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 16 Jun 2020 17:48:18 +0100 Subject: [PATCH] GTK: fix control flow in do_cmdline(). In commit 4ecc3f3c09 I did a knee-jerk fix of a macro of the form #define SECOND_PASS_ONLY { body; } on the grounds that it was syntax-unsafe, so I wrapped it in the standard do while(0): #define SECOND_PASS_ONLY do { body; } while (0) But in this case, that was a bogus transformation, because the body executed 'continue' with the intention of affecting the containing loop (outside the macro). Moreover, ten lines above the macro definition was a comment specifically explaining why it _couldn't_ be wrapped in do while (0) ! Since then I've come up with an alternative break-and-continue-proof wrapper for macros that are supposed to expand to something that's syntactically a C statement. So I've used that instead, and while I'm at it, fixed the neighbouring EXPECTS_ARG as well. Spotted by Coverity, and well spotted indeed! How embarrassing. --- unix/gtkmain.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/unix/gtkmain.c b/unix/gtkmain.c index e7a0eff3..ec8f7da4 100644 --- a/unix/gtkmain.c +++ b/unix/gtkmain.c @@ -315,22 +315,25 @@ bool do_cmdline(int argc, char **argv, bool do_everything, Conf *conf) char *val; /* - * Macros to make argument handling easier. Note that because - * they need to call `continue', they cannot be contained in - * the usual do {...} while (0) wrapper to make them - * syntactically single statements; hence it is not legal to - * use one of these macros as an unbraced statement between - * `if' and `else'. + * Macros to make argument handling easier. + * + * Note that because they need to call `continue', they cannot be + * contained in the usual do {...} while (0) wrapper to make them + * syntactically single statements. I use the alternative if (1) + * {...} else ((void)0). */ -#define EXPECTS_ARG { \ - if (--argc <= 0) { \ - err = true; \ - fprintf(stderr, "%s: %s expects an argument\n", appname, p); \ - continue; \ - } else \ - val = *++argv; \ -} -#define SECOND_PASS_ONLY do { if (!do_everything) continue; } while (0) +#define EXPECTS_ARG if (1) { \ + if (--argc <= 0) { \ + err = true; \ + fprintf(stderr, "%s: %s expects an argument\n", appname, p); \ + continue; \ + } else \ + val = *++argv; \ + } else ((void)0) +#define SECOND_PASS_ONLY if (1) { \ + if (!do_everything) \ + continue; \ + } else ((void)0) while (--argc > 0) { const char *p = *++argv;