lkml.org 
[lkml]   [2018]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2] reset: add support for non-DT systems
On Wed, Feb 14, 2018 at 10:59 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2018-02-13 20:17 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>> On Tue, Feb 13, 2018 at 8:39 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:

>>> + for (index = 0; lookup->dev; index++, lookup++) {
>>> + if (strcmp(dev_id, lookup->dev))
>>> + continue;
>>> +
>>> + if ((!id && !lookup->id) ||
>>> + (id && lookup->id && !strcmp(id, lookup->id))) {
>>> + rstc = __reset_control_get_internal(rcdev,
>>> + index, shared);
>>> + break;
>>> + }
>>
>> Wouldn't be slightly more readable
>>
>> if (id && lookup->id) {
>> if (strcmp(id, lookup->id))
>> continue;
>> } else if (id || lookup->id) {
>> continue;
>> }
>>
>> rstc = __reset_control_get_internal(rcdev, index, shared);
>> break;
>>
>
> You'd get less indentations, yes but I wanted to emphasize the
> condition on which we want to stop in this line.

It doesn't matter (indentation) here, esp. taking into consideration
that you already have another condition by which you continue the
loop.
My proposal adds consistency and from my point of view makes easier to parse.

You check all false conditions in a row, end up with a true one.

>>> + if (!rstc)
>>> + return optional ? NULL : ERR_PTR(-ENOENT);
>>
>> Isn't simpler
>>
>> if (!optional && !rstc) // or other way around, depending which might
>> be more offten
>> return ERR_PTR(...);
>>
>
> IMO it's just a matter of taste.

I think in my proposal it's more straight forward. Easier to parse by reader.

--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2018-02-14 12:33    [W:0.056 / U:2.908 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site