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 cleanupsACK 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list