linux-kernel - RE: [PATCH 1/2] Drivers: hv: vmbus: Remove duplication and cleanup code in create_gpadl

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: Fri, 12 Jan 2024 16:19:05 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Markus Elfring <Markus.Elfring@....de>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, Dexuan Cui <decui@...rosoft.com>, Haiyang
 Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dan Carpenter
	<dan.carpenter@...aro.org>, "kernel-janitors@...r.kernel.org"
	<kernel-janitors@...r.kernel.org>
CC: LKML <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/2] Drivers: hv: vmbus: Remove duplication and cleanup
 code in create_gpadl_header()

From: Markus Elfring <Markus.Elfring@....de> Sent: Friday, January 12, 2024 12:06 AM
> 
> …
> > Eliminate the duplication by making minor tweaks to the logic and
> > associated comments. While here, simplify the handling of memory
> > allocation errors, and use umin() instead of open coding it.
> …
> 
> I got the impression that the adjustment for the mentioned macro
> should be performed in a separate update step of the presented patch series.
> https://elixir.bootlin.com/linux/v6.7/source/include/linux/minmax.h#L95
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu
> mentation/process/submitting-patches.rst?h=v6.7#n81
> 

To me, this is a judgment call.  Breaking out the umin() change into
a separate patch is OK, but for consistency then I should probably
break out the change to memory allocation errors in the same
way.   Then we would have three patches, plus the patch to
separately handle the indentation so the changes are reviewable.
To me, that's overkill for updates to a single function that have
no functionality change.  The intent of the patch is to cleanup
and simplify a single 13-year old function, and it's OK to do
that in a single patch (plus the indentation patch).

Wei Liu is the maintainer for the Hyper-V code.  Wei -- any
objections to keeping a single patch (plus the indentation patch)?
But I'll break it out if that's your preference.

Michael