On 23.02.2018 08:51, Su Hang wrote: > Add brackets that wrap `if`, `else`, `while` that hold single > statements. > > In order to do this, I write a simple python regex script. > > Since then, all complaints rised by checkpatch.pl has been suppressed. > > Signed-off-by: Su Hang <suhan...@mails.ucas.ac.cn> > --- > util/uri.c | 462 > ++++++++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 291 insertions(+), 171 deletions(-) > > diff --git a/util/uri.c b/util/uri.c > index 278e876ab8b1..48f7298787b1 100644 > --- a/util/uri.c > +++ b/util/uri.c > @@ -105,18 +105,18 @@ static void uri_clean(URI *uri); > */ > > #define IS_UNWISE(p) > \ > - (((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) || > \ > - ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) || > \ > - ((*(p) == ']')) || ((*(p) == '`'))) > + (((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) || > \ > + ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) || > \ > + ((*(p) == ']')) || ((*(p) == '`'))) > /* > * reserved = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" | "$" | "," | > * "[" | "]" > */ > > #define IS_RESERVED(x) (((x) == ';') || ((x) == '/') || ((x) == '?') || > \ > - ((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') || > \ > - ((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') || > \ > - ((x) == ']')) > + ((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') || > \ > + ((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') || > \ > + ((x) == ']'))
The above whitespace changes should ideally be done in the first patch instead. > /* > * unreserved = alphanum | mark > @@ -211,15 +211,17 @@ static int rfc3986_parse_scheme(URI *uri, const char > **str) > { > const char *cur; > > - if (str == NULL) > + if (str == NULL) { > return -1; > + } > > cur = *str; > - if (!ISA_ALPHA(cur)) > + if (!ISA_ALPHA(cur)) { > return 2; > + } > cur++; > - while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || > - (*cur == '+') || (*cur == '-') || (*cur == '.')) > + while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == > '-') || > + (*cur == '.')) > cur++; You've changed the while statement, but checkpatch.pl apparently does not complain about missing curly braces here ... that's strange, I thought we'd also wanted to enforce curly braces for while loops. Anyway, could you please add curly braces around the "*cur++;" here, too? > @@ -1437,15 +1503,18 @@ done_cd: > /* string will overlap, do not use strcpy */ > tmp = cur; > segp += 3; > - while ((*tmp++ = *segp++) != 0) > + while ((*tmp++ = *segp++) != 0) { > ; > + } A bikeshed-painting-friday question for everybody on qemu-devel: Should there be a single semicolon inside curly braces in this case, or not? > /* If there are no previous segments, then keep going from here. */ > segp = cur; > - while ((segp > path) && ((--segp)[0] == '/')) > + while ((segp > path) && ((--segp)[0] == '/')) { > ; (dito) > - if (segp == path) > + } > + if (segp == path) { > continue; > + } > > /* "segp" is pointing to the end of a previous segment; find it's > * start. We need to back up to the previous segment and start [...] > @@ -1491,8 +1562,9 @@ done_cd: > static int is_hex(char c) > { > if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) || > - ((c >= 'A') && (c <= 'F'))) > + ((c >= 'A') && (c <= 'F'))) { > return 1; > + } > return 0; > } Not related to your patch, but an idea for a future clean-up patch: We've already got qemu_isxdigit(), so there is no real need for this separate is_hex() function. [...] > @@ -2020,17 +2127,19 @@ char *uri_resolve_relative(const char *uri, const > char *base) > */ > if (bptr[pos] != ref->path[pos]) { /* check for trivial URI == base > */ > for (; bptr[ix] != 0; ix++) { > - if (bptr[ix] == '/') > + if (bptr[ix] == '/') { > nbslash++; > + } > } > } > len = strlen(uptr) + 1; > } > > if (nbslash == 0) { > - if (uptr != NULL) > + if (uptr != NULL) { > /* exception characters from uri_to_string */ > - val = uri_string_escape(uptr, "/;&=+$,"); > + } > + val = uri_string_escape(uptr, "/;&=+$,"); That's a bug: uri_string_escape() should be within the curly braces instead. By the way, found that one with the following trick: Compare the disassemblies before and after you're changes: git checkout master make util/uri.o strip util/uri.o objdump -Drx util/uri.o > /tmp/uri-master.txt git checkout cleanupbranch make util/uri.o strip util/uri.o objdump -Drx util/uri.o > /tmp/uri-cleanup.txt diff -u /tmp/uri-*.txt Since you're only doing coding style clean-up, there should not be any diff in the resulting assembler code. Cheers, Thomas