Re: [PATCH v2 00/22] Remove unnecessary curly brackets around one-line bodies

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/13/2014 04:52 PM, John Ferlan wrote:
> 
> 
> On 11/13/2014 09:37 AM, Martin Kletzander wrote:
>> Brackets were removed by the following script in the root directory of
>> libvirt's repository:
>>
>>   for i in $(git ls-files | grep '\.[ch]$'); do
>>     echo -n "$i ... ";
>>     emacs $i --batch --eval \
>>       '(replace-regexp "^\\([[:space:]]*\\(if\\|for\\|while\\).*)\\)[[:space:]]*{\\(\n[^\n;]*;[^\n;]*\\)\n[[:space:]]*}$" "\\1\\3" nil (point-min) (point-max))' \
>>     -f save-buffer &>/dev/null;
>>     echo done;
>>   done
>>
>> v2:
>>  - Smaller patches, so there has to be none allowed through by
>>    moderators
>>  - Only curly brackets around one-line bodies with one-line preceding
>>    conditions removed
>>  - syntax-check! (with some other bracket-spacing.pl cleanups
>>
> 
> ACK series in general - although something needs to be done about 17/22
> and the comment in 22/22 probably needs an adjustment as well as a way
> hopefully to indicate what rule is being violated.

I have not closely reviewed the series, and the perl in patch 22/22 is
complex enough that I don't feel I have enough time to do it justice in
a quick review today.  I'll trust John's review - he did have a good
point about multiline macro bodies needing do {} while (0) guards -
maybe we add yet another syntax check for that?  I'm guessing you wrote
the series by applying patch 22 first, then fixing in pieces until
things work; so if 22 is good and syntax-check passes, then all the
other patches prove we silenced the syntax problems (but not necessarily
that the transformations to silence the problems were correct, as was
the case with macros in tools/).

At any rate, this series will be a conflict magnet to other pending
patches, so if we're going to take it, I don't want to hold it up for a
long time.  The fact that you have a syntax-check to enforce things is a
huge improvement over the v1 proposal.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]