Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1721912

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

From Andy Shevchenko <andy.shevchenko@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH V3] acpi: apei: clear error status before acknowledging the error
Date 2017-08-28 21:20 +0200
Message-ID <ujxJM-13g-23@gated-at.bofh.it> (permalink)
References (2 earlier) <ujvRD-8lV-11@gated-at.bofh.it> <ujw1k-8pb-21@gated-at.bofh.it> <ujw1l-8pb-35@gated-at.bofh.it> <ujwkG-8vp-11@gated-at.bofh.it> <ujwXo-vG-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH V3] acpi: apei: clear error status before acknowledging the error Tyler Baicar <tbaicar@codeaurora.org> - 2017-08-28 19:00 +0200
  Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-08-28 19:10 +0200
    Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Borislav Petkov <bp@suse.de> - 2017-08-28 19:20 +0200
      Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-28 19:30 +0200
        Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Borislav Petkov <bp@suse.de> - 2017-08-28 19:30 +0200
          Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-28 19:50 +0200
            Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Borislav Petkov <bp@suse.de> - 2017-08-28 20:30 +0200
              Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-28 21:20 +0200
              Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-08-28 21:20 +0200
  Re: [PATCH V3] acpi: apei: clear error status before acknowledging  the error Borislav Petkov <bp@alien8.de> - 2017-08-29 10:20 +0200

csiph-web