Re: [PATCH] module: Fix implicit type conversion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 08, 2021 at 07:31:05PM +0100, Miroslav Benes wrote:
> [CCing Luis]
> 
> Hi,
> 
> On Fri, 29 Oct 2021, Jiasheng Jiang wrote:
> 
> > The variable 'cpu' is defined as unsigned int.
> > However in the for_each_possible_cpu, its values is assigned to -1.
> > That doesn't make sense and in the cpumask_next() it is implicitly
> > type conversed to int.
> > It is universally accepted that the implicit type conversion is
> > terrible.
> > Also, having the good programming custom will set an example for
> > others.
> > Thus, it might be better to change the definition of 'cpu' from
> > unsigned int to int.
> 
> Frankly, I don't see a benefit of changing this. It seems fine to me. 
> Moreover this is not, by far, the only place in the kernel with the same 
> pattern.
> 
> Miroslav
> 
> > Fixes: 10fad5e ("percpu, module: implement and use is_kernel/module_percpu_address()")
> > Signed-off-by: Jiasheng Jiang <jiasheng@xxxxxxxxxxx>
> > ---
> >  kernel/module.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 927d46c..f10d611 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -632,7 +632,7 @@ static void percpu_modcopy(struct module *mod,
> >  bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr)
> >  {
> >  	struct module *mod;
> > -	unsigned int cpu;
> > +	int cpu;
> >  
> >  	preempt_disable();

If we're going to do this we we must ask, is it really worth it and
moving forward then add a semantic patch rule which will pick up on
misuses.

@ finds_for_each_cpu_unsigned_int @
identifier x;
iterator name for_each_possible_cpu;
iterator name for_each_online_cpu;
iterator name for_each_present_cpu;
statement S;
@@

-unsigned
int x;
<+...
(
for_each_possible_cpu(x) S
|
for_each_online_cpu(x) S
|
for_each_present_cpu(x) S
)
...+>


This produces:

 arch/arm/mm/cache-b15-rac.c                        |  2 +-
 arch/arm/mm/cache-uniphier.c                       |  2 +-
 arch/arm64/kernel/mte.c                            |  2 +-
 arch/arm64/kernel/smp.c                            |  2 +-
 arch/arm64/kvm/arm.c                               |  2 +-
 arch/ia64/kernel/smp.c                             |  2 +-
 arch/ia64/kernel/topology.c                        |  2 +-
 arch/ia64/mm/contig.c                              |  4 ++--
 arch/mips/kernel/mips-cm.c                         |  2 +-
 arch/mips/kernel/mips-cpc.c                        |  2 +-
 arch/mips/kernel/smp.c                             |  6 +++---
 arch/mips/mm/init.c                                |  2 +-
 arch/openrisc/kernel/smp.c                         |  2 +-
 arch/parisc/kernel/topology.c                      |  2 +-
 arch/powerpc/kernel/cacheinfo.c                    |  4 ++--
 arch/powerpc/kernel/iommu.c                        |  2 +-
 arch/powerpc/kernel/setup_32.c                     |  4 ++--
 arch/powerpc/kernel/setup_64.c                     |  8 ++++----
 arch/powerpc/kernel/smp.c                          |  2 +-
 arch/powerpc/mm/numa.c                             |  2 +-
 arch/powerpc/platforms/ps3/interrupt.c             |  2 +-
 arch/powerpc/platforms/pseries/hotplug-cpu.c       |  6 +++---
 arch/powerpc/platforms/pseries/mobility.c          |  2 +-
 arch/powerpc/platforms/pseries/setup.c             |  2 +-
 arch/s390/pci/pci_irq.c                            |  4 ++--
 arch/sh/kernel/topology.c                          |  2 +-
 arch/sh/mm/cache-j2.c                              |  6 +++---
 arch/sparc/kernel/iommu-common.c                   |  2 +-
 arch/sparc/kernel/smp_64.c                         |  4 ++--
 arch/x86/events/amd/uncore.c                       |  4 ++--
 arch/x86/kernel/apic/apic_numachip.c               |  2 +-
 arch/x86/kernel/apic/bigsmp_32.c                   |  2 +-
 arch/x86/kernel/apic/x2apic_cluster.c              |  2 +-
 arch/x86/kernel/apic/x2apic_uv_x.c                 |  2 +-
 arch/x86/kernel/cpu/mce/apei.c                     |  2 +-
 arch/x86/kernel/cpu/microcode/core.c               |  2 +-
 arch/x86/kernel/setup_percpu.c                     |  4 ++--
 arch/x86/kernel/smpboot.c                          |  4 ++--
 arch/x86/mm/cpu_entry_area.c                       |  2 +-
 arch/x86/mm/pti.c                                  |  2 +-
 arch/x86/xen/mmu_pv.c                              |  2 +-
 arch/x86/xen/smp_pv.c                              |  4 ++--
 arch/xtensa/kernel/irq.c                           |  2 +-
 arch/xtensa/kernel/smp.c                           |  4 ++--
 arch/xtensa/kernel/traps.c                         |  2 +-
 drivers/base/arch_numa.c                           |  2 +-
 drivers/base/arch_topology.c                       |  2 +-
 drivers/block/rnbd/rnbd-clt.c                      |  2 +-
 drivers/cpufreq/acpi-cpufreq.c                     |  4 ++--
 drivers/cpufreq/cpufreq_ondemand.c                 |  2 +-
 drivers/cpufreq/intel_pstate.c                     |  2 +-
 drivers/cpufreq/qcom-cpufreq-nvmem.c               |  4 ++--
 drivers/cpufreq/sun50i-cpufreq-nvmem.c             |  4 ++--
 drivers/idle/intel_idle.c                          |  2 +-
 drivers/iommu/iova.c                               |  4 ++--
 drivers/irqchip/irq-bcm6345-l1.c                   |  2 +-
 drivers/irqchip/irq-gic.c                          |  2 +-
 drivers/irqchip/irq-jcore-aic.c                    |  2 +-
 drivers/irqchip/irq-mips-gic.c                     |  2 +-
 drivers/macintosh/rack-meter.c                     |  2 +-
 drivers/net/ethernet/chelsio/libcxgb/libcxgb_ppm.c |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c      |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |  2 +-
 drivers/net/ethernet/marvell/mvneta.c              |  2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c    |  4 ++--
 drivers/pci/controller/pcie-iproc-msi.c            |  2 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c                  |  2 +-
 drivers/scsi/bnx2i/bnx2i_init.c                    |  2 +-
 drivers/scsi/bnx2i/bnx2i_iscsi.c                   |  2 +-
 drivers/scsi/fcoe/fcoe.c                           |  6 +++---
 drivers/scsi/fcoe/fcoe_transport.c                 |  2 +-
 drivers/scsi/libfc/fc_exch.c                       |  4 ++--
 drivers/scsi/libfc/fc_lport.c                      |  2 +-
 drivers/soc/bcm/brcmstb/biuctrl.c                  |  2 +-
 drivers/xen/events/events_base.c                   |  2 +-
 drivers/xen/events/events_fifo.c                   |  2 +-
 fs/fscache/main.c                                  |  2 +-
 kernel/cpu.c                                       |  4 ++--
 kernel/livepatch/transition.c                      | 12 ++++++------
 kernel/module.c                                    |  2 +-
 kernel/relay.c                                     |  8 ++++----
 kernel/sched/deadline.c                            |  2 +-
 kernel/sched/rt.c                                  |  2 +-
 kernel/smpboot.c                                   |  4 ++--
 kernel/stop_machine.c                              |  2 +-
 kernel/taskstats.c                                 |  2 +-
 kernel/trace/trace_hwlat.c                         |  4 ++--
 lib/cpu_rmap.c                                     |  6 +++---
 lib/test_lockup.c                                  |  2 +-
 mm/kmemleak.c                                      |  4 ++--
 mm/percpu-vm.c                                     |  4 ++--
 mm/percpu.c                                        |  8 ++++----
 mm/slub.c                                          |  2 +-
 mm/swap_slots.c                                    |  2 +-
 net/core/dev.c                                     |  2 +-
 net/ipv4/netfilter/arp_tables.c                    |  2 +-
 net/ipv4/netfilter/ip_tables.c                     |  2 +-
 net/ipv4/route.c                                   |  2 +-
 net/ipv6/netfilter/ip6_tables.c                    |  2 +-
 net/netfilter/x_tables.c                           |  4 ++--
 net/rds/page.c                                     |  2 +-
 net/sunrpc/svc.c                                   |  2 +-
 102 files changed, 148 insertions(+), 148 deletions(-)



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux