Messages in this thread | | | Date | Tue, 25 Nov 2014 13:58:06 -0800 | From | Darren Hart <> | Subject | Re: [PATCH 1/3] platform: x86: dell-rbtn: Dell Airplane Mode Switch driver |
| |
On Fri, Nov 28, 2014 at 01:54:57PM +0200, Mika Westerberg wrote: > On Fri, Nov 28, 2014 at 12:45:55PM +0100, Pali Rohár wrote: > > Hello, > > > > I will fix all those style problems and add some comments. > > > > On Friday 28 November 2014 12:33:28 Mika Westerberg wrote: > > > > + if (ACPI_FAILURE(status)) > > > > + return; > > > > + > > > > + rfkill_set_states(rfkill, !output, !output); > > > > > > You can also write it like: > > > > > > if (ACPI_SUCCESS(status)) > > > rfkill_set_states(rfkill, !output, !output); > > > > > > which looks better to me at least. > > > > > > > In whole module I'm using this style: > > > > f1(); > > if (f1_failed) > > return; > > f2() > > if (f2_failed) > > return; > > > > So I would like not to change it for one function. > > Fair enough.
And, in my opinion, it is better to test for errors than to test for success. This keeps the main logic out of a nested block. Not so critical here, but a good rule of thumb.
-- Darren Hart Intel Open Source Technology Center
| |