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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 28 Aug 2017 22:14:29 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Borislav Petkov <bp@...e.de>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Tyler Baicar <tbaicar@...eaurora.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, Will Deacon <will.deacon@....com>,
        james.morse@....com, shiju.jose@...wei.com,
        Geliang Tang <geliangtang@...il.com>,
        Tony Luck <tony.luck@...el.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3] acpi: apei: clear error status before acknowledging
 the error

On Mon, Aug 28, 2017 at 9:27 PM, Borislav Petkov <bp@...e.de> 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