On Wed, Aug 06, 2014 at 08:05:46AM +0200, Markus Armbruster wrote: > "Gonglei (Arei)" <arei.gong...@huawei.com> writes: > > > Hi, > > > >> > > >> > $WHATEVER: don't use 'Yoda conditions' > >> > > >> > 'Yoda conditions' are not part of idiomatic QEMU coding > >> > style, so rewrite them in the more usual order. > >> > >> > >> OK but why stop at these files? How about this > >> instead? > >> > > I just search c files by using key words like "NULL ==" etc. > > > > I don't think we should change conditional statements like ">" and ">=". > > Eric pointed out it's actually incorrect for NaNs. > > If you want to touch inequalities, separate patch(es) please, because > they need more thorough review, both for correctness and for style. > > > BTW, just using like "value == NULL" instead of "NULL == value" in all files > > is not a good idea, which we have discussed in my patch serials v2. So, I > > posted > > v3, add change log " imitate nearby code about using '!value' or > > value == NULL' at > > every patch " . > > Re "not a good idea": I think rewriting "NULL == value" to "value == > NULL" *is* a good idea, but rewriting it to "!value" where that blends > in with surrounding code is a *better* idea. > > Gonglei's patches do that, Michael's don't, but are more complete. > Therefore:
Yes but it's unrelated to Yoda: we have x != NULL without Yoda in a lot of places. So this seems, to me, an unrelated issue. If people feel this == NULL -> !x is desired, it's better to do it all at once IMHO, and do x != NULL -> x at the same time. Easy to run another script to do it on top. > > > So, maybe you can post patches for those files I have missed in the > > serials, > > but not simply instead all by semantic script IMO, thanks! > > Easy: apply Gonglei's patches before you run the script. > > You may have to split patches along subsystem boundaries to get them in. > Bothersome, as it involves guessing boundaries. Not a request from me, > just a warning of possible misfortune :) It's going in through trivial tree, I don't think split-up is necessary.