| Message ID | 20250221-rmv_return-v1-0-cc8dff275827@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove weird and needless 'return' for void APIs | expand |
On Fri, Feb 21, 2025 at 05:02:05AM -0800, Zijun Hu wrote: > This patch series is to remove weird and needless 'return' for > void APIs under include/ with the following pattern: > > api_header.h: > > void api_func_a(...); > > static inline void api_func_b(...) > { > return api_func_a(...); > } > > Remove the needless 'return' in api_func_b(). > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > --- > Zijun Hu (18): > mm/mmu_gather: Remove needless return in void API tlb_remove_page() > cpu: Remove needless return in void API suspend_enable_secondary_cpus() > crypto: api - Remove needless return in void API crypto_free_tfm() > crypto: scomp - Remove needless return in void API crypto_scomp_free_ctx() > sysfs: Remove needless return in void API sysfs_enable_ns() > skbuff: Remove needless return in void API consume_skb() > wifi: mac80211: Remove needless return in void API _ieee80211_hw_set() > net: sched: Remove needless return in void API qdisc_watchdog_schedule_ns() > ipv4/igmp: Remove needless return in void API ip_mc_dec_group() > IB/rdmavt: Remove needless return in void API rvt_mod_retry_timer() > ratelimit: Remove needless return in void API ratelimit_default_init() > siox: Remove needless return in void API siox_driver_unregister() > gpiolib: Remove needless return in two void APIs > PM: wakeup: Remove needless return in three void APIs > mfd: db8500-prcmu: Remove needless return in three void APIs > rhashtable: Remove needless return in three void APIs > dma-mapping: Remove needless return in five void APIs > mtd: nand: Do not return void function in void function These span loads of different subsystems, please just submit them all to the different subsystems directly, not as one big patch series which none of us could take individually. thanks, greg k-h
On 2/21/2025 9:12 PM, Greg Kroah-Hartman wrote: > These span loads of different subsystems, please just submit them all to > the different subsystems directly, not as one big patch series which > none of us could take individually. > sure. will do it as you suggest. thank you. > thanks, > > greg k-h
On Fri, Feb 21, 2025, at 14:02, Zijun Hu wrote: > This patch series is to remove weird and needless 'return' for > void APIs under include/ with the following pattern: > > api_header.h: > > void api_func_a(...); > > static inline void api_func_b(...) > { > return api_func_a(...); > } > > Remove the needless 'return' in api_func_b(). > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> I have no objection to the changes, but I think you should describe the motivation for them beyond them being 'weird'. Do these 'return' statements get in the way of some other work you are doing? Is there a compiler warning you want to enable to ensure they don't come back? Is this all of the instances in the kernel or just the ones you found by inspection? Arnd
On Fri, 21 Feb 2025 05:02:05 -0800 Zijun Hu <quic_zijuhu@quicinc.com> wrote: > This patch series is to remove weird and needless 'return' for > void APIs under include/ with the following pattern: > > api_header.h: > > void api_func_a(...); > > static inline void api_func_b(...) > { > return api_func_a(...); > } > > Remove the needless 'return' in api_func_b(). > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > --- > Zijun Hu (18): > mm/mmu_gather: Remove needless return in void API tlb_remove_page() > cpu: Remove needless return in void API suspend_enable_secondary_cpus() > crypto: api - Remove needless return in void API crypto_free_tfm() > crypto: scomp - Remove needless return in void API crypto_scomp_free_ctx() > sysfs: Remove needless return in void API sysfs_enable_ns() > skbuff: Remove needless return in void API consume_skb() > wifi: mac80211: Remove needless return in void API _ieee80211_hw_set() > net: sched: Remove needless return in void API qdisc_watchdog_schedule_ns() > ipv4/igmp: Remove needless return in void API ip_mc_dec_group() > IB/rdmavt: Remove needless return in void API rvt_mod_retry_timer() > ratelimit: Remove needless return in void API ratelimit_default_init() > siox: Remove needless return in void API siox_driver_unregister() > gpiolib: Remove needless return in two void APIs > PM: wakeup: Remove needless return in three void APIs > mfd: db8500-prcmu: Remove needless return in three void APIs > rhashtable: Remove needless return in three void APIs > dma-mapping: Remove needless return in five void APIs > mtd: nand: Do not return void function in void function > > include/asm-generic/tlb.h | 2 +- > include/crypto/internal/scompress.h | 2 +- > include/linux/cpu.h | 2 +- > include/linux/crypto.h | 2 +- > include/linux/dma-mapping.h | 12 ++++++------ > include/linux/gpio.h | 4 ++-- > include/linux/igmp.h | 2 +- > include/linux/mfd/dbx500-prcmu.h | 6 +++--- > include/linux/mtd/nand.h | 18 ++++++++++++------ > include/linux/pm_wakeup.h | 6 +++--- > include/linux/ratelimit.h | 4 ++-- > include/linux/rhashtable.h | 6 +++--- > include/linux/siox.h | 2 +- > include/linux/skbuff.h | 2 +- > include/linux/sysfs.h | 2 +- > include/net/mac80211.h | 2 +- > include/net/pkt_sched.h | 2 +- > include/rdma/rdmavt_qp.h | 2 +- > 18 files changed, 42 insertions(+), 36 deletions(-) > --- > base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b > change-id: 20250221-rmv_return-f1dc82d492f0 > > Best regards, Is this something that could be done with a coccinelle script?
On Fri, 2025-02-21 at 11:00 -0800, Stephen Hemminger wrote: > > Is this something that could be done with a coccinelle script? > Almost enough to do this: @@ identifier fn; expression E; @@ void fn(...) { ... -return E; } It takes a long time to run though, and does some wrong things as well: if the return is in the middle of the function, it still matches and removes it erroneously. johannes
On 2025/2/21 23:40, Arnd Bergmann wrote: >> This patch series is to remove weird and needless 'return' for >> void APIs under include/ with the following pattern: >> >> api_header.h: >> >> void api_func_a(...); >> >> static inline void api_func_b(...) >> { >> return api_func_a(...); >> } >> >> Remove the needless 'return' in api_func_b(). >> >> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > I have no objection to the changes, but I think you should > describe the motivation for them beyond them being 'weird'. > yes. C spec such as C17 have this description about return statement: 6.8.6.4: A return statement with an expression shall not appear in a function whose return type is void. A return statement without an expression shall only appear in a function whose return type is void. do we need to treat below return statement as bad code style ? void api_func_a(...); void api_func_b(...) { ... return api_func_a(...); // return void function in void func ... } > Do these 'return' statements get in the way of some other > work you are doing? Is there a compiler warning you want > to enable to ensure they don't come back? Is this all of > the instances in the kernel or just the ones you found by > inspection? actually, i find this weird return usage by reading code by accident in driver core firstly: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-testing&id=a44073c28bc6d4118891d61e31c9fa9dc4333dc0 then i check folder include/ and work out this patch series. not sure if there are still such instances in current kernel tree.
On 2025/2/22 03:36, Johannes Berg wrote: > On Fri, 2025-02-21 at 11:00 -0800, Stephen Hemminger wrote: >> Is this something that could be done with a coccinelle script? >> > Almost enough to do this: > > @@ > identifier fn; > expression E; > @@ > void fn(...) > { > ... > -return > E; > } > > > It takes a long time to run though, and does some wrong things as well: > if the return is in the middle of the function, it still matches and > removes it erroneously. if return is in the middle, we may need to convert the return statement in to two statement as [PATCH 18/18] does: https://lore.kernel.org/all/20250221-rmv_return-v1-18-cc8dff275827@quicinc.com/ namely, Convert "return func(...);" to "func(...); return;" C spec such as C17 have this description about return statement: 6.8.6.4: A return statement with an expression shall not appear in a function whose return type is void. A return statement without an expression shall only appear in a function whose return type is void. so, do we need to treat "return void function in void function" as bad code style and make coccinelle script check this bad usage?
This patch series is to remove weird and needless 'return' for void APIs under include/ with the following pattern: api_header.h: void api_func_a(...); static inline void api_func_b(...) { return api_func_a(...); } Remove the needless 'return' in api_func_b(). Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> --- Zijun Hu (18): mm/mmu_gather: Remove needless return in void API tlb_remove_page() cpu: Remove needless return in void API suspend_enable_secondary_cpus() crypto: api - Remove needless return in void API crypto_free_tfm() crypto: scomp - Remove needless return in void API crypto_scomp_free_ctx() sysfs: Remove needless return in void API sysfs_enable_ns() skbuff: Remove needless return in void API consume_skb() wifi: mac80211: Remove needless return in void API _ieee80211_hw_set() net: sched: Remove needless return in void API qdisc_watchdog_schedule_ns() ipv4/igmp: Remove needless return in void API ip_mc_dec_group() IB/rdmavt: Remove needless return in void API rvt_mod_retry_timer() ratelimit: Remove needless return in void API ratelimit_default_init() siox: Remove needless return in void API siox_driver_unregister() gpiolib: Remove needless return in two void APIs PM: wakeup: Remove needless return in three void APIs mfd: db8500-prcmu: Remove needless return in three void APIs rhashtable: Remove needless return in three void APIs dma-mapping: Remove needless return in five void APIs mtd: nand: Do not return void function in void function include/asm-generic/tlb.h | 2 +- include/crypto/internal/scompress.h | 2 +- include/linux/cpu.h | 2 +- include/linux/crypto.h | 2 +- include/linux/dma-mapping.h | 12 ++++++------ include/linux/gpio.h | 4 ++-- include/linux/igmp.h | 2 +- include/linux/mfd/dbx500-prcmu.h | 6 +++--- include/linux/mtd/nand.h | 18 ++++++++++++------ include/linux/pm_wakeup.h | 6 +++--- include/linux/ratelimit.h | 4 ++-- include/linux/rhashtable.h | 6 +++--- include/linux/siox.h | 2 +- include/linux/skbuff.h | 2 +- include/linux/sysfs.h | 2 +- include/net/mac80211.h | 2 +- include/net/pkt_sched.h | 2 +- include/rdma/rdmavt_qp.h | 2 +- 18 files changed, 42 insertions(+), 36 deletions(-) --- base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b change-id: 20250221-rmv_return-f1dc82d492f0 Best regards,