linux-kernel - Re: [PATCH 1/3] watchdog: s3c2410_wdt: Fold s3c2410_get_wdt_drv

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, 6 Mar 2023 09:17:23 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Alim Akhtar <alim.akhtar@...sung.com>,
        linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org, kernel@...gutronix.de,
        linux-kernel@...r.kernel.org,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        linux-watchdog@...r.kernel.org
Subject: Re: [PATCH 1/3] watchdog: s3c2410_wdt: Fold
 s3c2410_get_wdt_drv_data() into only caller

On 3/6/23 01:09, Uwe Kleine-König wrote:
> s3c2410_get_wdt_drv_data() is only called by s3c2410wdt_probe(), so the
> implementation of the former can move to the latter.
> 
> scripts/bloat-o-meter reports for this change (on an ARCH=arm
> allmodconfig build):
> 
> 	add/remove: 1/1 grow/shrink: 0/2 up/down: 4/-129 (-125)
> 

The reason for separating functions in this case wasn't that the separate function
would be called several times. It was to improve code readability. If anything,
I would argue that it might sense to split the already lengthy probe function
further instead of combining it.

Maybe I am old fashioned. Maybe the old "split your code into multiple functions
if the function size is larger than X lines" no longer applies, and it is now
"never split functions unless the separated function is called more than once".
Still, I am quite concerned that accepting this patch would result in a flurry
of similar patches which would all do nothing but hurt readability, using the
same set of arguments. I really don't like where this is going. I am going to
leave it up to Wim to decide if and how to proceed.

Guenter