linux-kernel - Re: [PATCH v2 2/3] platform/x86: intel_scu_ipc: Simplify code with cleanup helpers

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 for Android: free password hash cracker in your pocket

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

Message-ID: <e4ede6f5-6459-862a-adb9-fdd6f9524c51@linux.intel.com>
Date: Mon, 21 Oct 2024 13:08:55 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
    LKML <linux-kernel@...r.kernel.org>, platform-driver-x86@...r.kernel.org, 
    Andy Shevchenko <andy@...nel.org>, 
    Mika Westerberg <mika.westerberg@...ux.intel.com>, 
    Hans de Goede <hdegoede@...hat.com>, Ferry Toth <fntoth@...il.com>
Subject: Re: [PATCH v2 2/3] platform/x86: intel_scu_ipc: Simplify code with
 cleanup helpers

On Mon, 21 Oct 2024, Andy Shevchenko wrote:

> On Mon, Oct 21, 2024 at 12:32 PM Ilpo Järvinen
> <ilpo.jarvinen@...ux.intel.com> wrote:
> > On Mon, 21 Oct 2024, Andy Shevchenko wrote:
> 
> ...
> 
> > IMO, this change is doing too many things at once and it's hard to justify
> > why those changes must be kept in the same patch. If the guard() change
> > is done first and only then the logic reversions, both patches would
> > probably be near trivial to review for correctness.
> 
> Are you insisting on this?
> Because that's how I have done similar changes in the past all over
> the kernel, and IIRC you are the first one asking for this :-)

Well, I know I could go through the patch as is and likely find out it's 
correct. But as is, it requires clearly more effort that it would if those 
two things would be separated. The contexts would be much smaller and 
focused if you split this into two and since you know the end result (the 
current patch), the second patch is just the diff of the first to it.

I'm not saying it's always required but TBH, this patch definitely would 
get simpler to read if you split it into two. So to answer your question, 
it's more of a judgement call than insisting it always.

-- 
 i.