Re: [Qemu-devel] [PATCH v3 3/3] util/uri.c: add brackets to `if` statements

[苏航]

Thanks for your comments :)
I will pay more attention to what you point out.

"Eric Blake" <address@hidden>wrote:
> On 02/23/2018 03:34 AM, Thomas Huth wrote:
> > 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.
> 
> Without documenting the script, no one else can reproduce this; but it's 
> no different than if they had manually made changes instead of trying to 
> script it, so I'm not sure this sentence adds much in its current form.
> 
> >>
> >> Since then, all complaints rised by checkpatch.pl has been suppressed.
> 
> s/rised/raised/
> s/Since then,/With that/
> s/has/have/
> 
> >>
> >> Signed-off-by: Su Hang <address@hidden>
> >> ---
> >>   util/uri.c | 462 
> >> ++++++++++++++++++++++++++++++++++++++-----------------------
> >>   1 file changed, 291 insertions(+), 171 deletions(-)
> >>
> 
> >>       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.
> 
> Maybe because it gets lost since the condition expanded over more than 
> one line?  But yes, now that we've noticed it manually, it should be 
> fixed.  While at it, you can avoid the redundant ():
> 
> while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || *cur == '+' || *cur == '-' ||
>         *cur == '.') {
> 
> 
> >> -        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?
> > 
> 
> Checkpatch doesn't complain, but lone ';' statements are rare.  I'd omit 
> it, and use just:
> 
> while (cond) {
> }
> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org