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

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.