Re: [PATCH V3] acpi: apei: clear error status before acknowledging the error

[Andy Shevchenko]

On Mon, Aug 28, 2017 at 9:27 PM, Borislav Petkov <bp@xxxxxxx> wrote:
> On Mon, Aug 28, 2017 at 08:44:21PM +0300, Andy Shevchenko wrote:
>> For my opinion, since you asked, the either case needs a comment on
>> top of that additional check.
>
> That's because the comment belongs to the v2 part of the check.

Sorry not being clear, I meant another, separate comment.

>> Separate conditionals in independent cases are, of course, better.
>
> Yes, and separate are easier to read if you read them like this:
>
> +       if (rc == -ENOENT)
> +               return rc;
>
> <--- Ok, we got the missing entry out of the way, now, here, we have a
> valid entry. Now we can concentrate on processing it further.
>
>         ... other check and ack and ...
>
> And this becomes a lot more natural when you're staring at a big function
> which does a lot of things and you concentrate only on the main path.
>
> Oh, and this is how those checks get translated to asm as there you
> don't really have compound if-statements. So if you switch your mind to
> reading such checks separately, you're practically ready to read their
> asm translation too...
>
> Anyway, this is how I see it.

Looking into commit message again I think the word 'also' creates all
this. Two separate commits might be perfect, though good enough is to
have an additional comment to the new check.

Thanks for sharing detailed point of view.

-- 
With Best Regards,
Andy Shevchenko