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

[Martin Kletzander]

On Thu, Nov 13, 2014 at 05:09:04PM -0700, Eric Blake wrote:
>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

I wouldn't believe we have multi-line macros without its own body.
Anyway, I fixed that.

I started working on the syntax-check for multiline macros without
do-while block, but it's going to be a bit painful.

>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/).
>

Actually, no.  I had few variations of that syntax-check, but the only
thing I was pretty sure about was that (regexp-replace) I did.  Then
it was just a matter of splitting it into small enough chunks so it
doesn't have to be allowed to list by moderator and so it makes sense.

>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.
>

I fixed everything and after discussion with John pushed the fixed
versions, thank you for spotting all the details.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141114/926df071/attachment-0001.sig>