From mboxrd@z Thu Jan 1 00:00:00 1970 From: NeilBrown Subject: Re: [PATCH] kernel.h: Add for_each_if() Date: Sat, 14 Jul 2018 09:37:25 +1000 Message-ID: <87bmbavhai.fsf@notabene.neil.brown.name> References: <20180709083650.23549-1-daniel.vetter@ffwll.ch> <20180709162509.29343-1-daniel.vetter@ffwll.ch> <20180709163001.8fb8148223a57bc46a13fbda@linux-foundation.org> <20180710075328.GG3008@phenom.ffwll.local> <871scbwfd4.fsf@notabene.neil.brown.name> <20180711160547.59f086a587c7f3c8d3c40f0f@linux-foundation.org> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Return-path: In-Reply-To: <20180711160547.59f086a587c7f3c8d3c40f0f@linux-foundation.org> Sender: linux-kernel-owner@vger.kernel.org To: Andrew Morton , Daniel Vetter Cc: LKML , DRI Development , Intel Graphics Development , Gustavo Padovan , Maarten Lankhorst , Sean Paul , David Airlie , Kees Cook , Ingo Molnar , Greg Kroah-Hartman , Wei Wang , Stefan Agner , Andrei Vagin , Randy Dunlap , Andy Shevchenko , Yisheng Xie , Peter Zijlstra , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org --=-=-= Content-Type: text/plain On Wed, Jul 11 2018, Andrew Morton wrote: > On Wed, 11 Jul 2018 13:51:08 +0200 Daniel Vetter wrote: > >> But I still have the situation that a bunch of maintainers acked this >> and Andrew Morton defacto nacked it, which I guess means I'll keep the >> macro in drm? The common way to go about this seems to be to just push >> the patch series with the ack in some pull request to Linus and ignore >> the people who raised questions, but not really my thing. > > Heh. > > But, am I wrong? Code which uses regular kernel style doesn't have > these issues. We shouldn't be enabling irregular style - we should be > making such sites more regular. The fact that the compiler generates a > nice warning in some cases simply helps us with that. I think you are wrong .... or at least, not completely correct. I think it is perfectly acceptable in Linux to have code like: for (....) if (x) something(); else something_else(); Would you agree? If not, then I'm the one who is wrong. Otherwise.... The problem is that for certain poorly written for_each_foo() macros, such as blkg_for_each_descendant_pre() (and several others identified in this patch series), writing blkg_for_each_descendant_pre(...) if (x) something(); else something_else(); will trigger a compiler warning. This is inconsistent with the behaviour of a simple "for". So I do think that the macros should be fixed, and I don't think that sprinkling extra braces is an appropriate response. I'm not personally convinced that writing if_no_else(cond) is easier than just writing if (!(cond)); else in these macros, but I do think that the macros should be fixed and maybe this is the path-of-least-resistance to getting it done. Thanks, NeilBrown --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAltJN7cACgkQOeye3VZi gblHDhAAj/rcY3vyWl833/lkI0CKhD84dktXAFOnagQGDe8kEJi/EfOhhxiii5Rm dTwWsDlSrb0IN0/bZBDcKavMik5RJZRpOPaO/qFZaXwKLVQbjKZGpX8PMraUIzLK bvlRdhO73I83/2gzKJmzhvkUFVmUA2LXoFCiU2O4UP+JtPvo1CduzgbP5PjAZ0ca IC7ew+NRnj9nbEo8eeqJMoteJNz2+lEzWRBGn1NJ3vTKM4cPPUy7P7EHdZkzuoba DBzoRroXJzjP1d+CmG8XmEWwr+CII2o0uv+kO6fGiLFZNF4EXxT3OnBfz7IS051t UO4+PJVR8CzgsSl46NHTl14mvYgO+aLIz6S4+aK7RIQr5FXqk6GXMGsT9b6XtUdO ebBIoYNmlbgyqjLPaq218+XfTeLfxwrNaBKBt7z5Qrt6oVkv4i0+qCThe0VgNrtp uQMBJwLOojdz6mXnikqlHKzjDiD3fnMlxznFsgFWLk1Jt7/5miYHBjnA02H6YR/w mqE4Fkutn1q8CQch80Ju/hMlwmll+NfWy8iRdCcn85LfbJYTqRsDpcCA/DodO0al hpLZDUkA7io33jLFCDChqO6rfL+fE9Y4NYFSXpc/rmNe2F9fZZqEiwO24THx9UH4 kxjVnr5bQYMyxlU6HxALsNrd4tps1hhYhHhzok9pT0OddP1DrEM= =OSfx -----END PGP SIGNATURE----- --=-=-=--