Messages in this thread | | | Date | Mon, 6 Mar 2023 09:17:23 -0800 | From | Guenter Roeck <> | 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
| |