On Saturday, April 16, 2016 01:25:09 PM Pedro Giffuni wrote:
Hello;
Using coccinelle, and some hand re-formatting, I generated a patch to
make use of the nitems() macro in sys, which is too big for
phabricator [1].
I was careful to exclude anything from the contrib directory or
any code that is shared with userland (as to not have to include
additional headers).
The patch is big but pretty safe, I think. The changes are small but
still it touches many files[1].
I would like some feedback on the convenience of doing such replacement.
If it is a good idea we could do the same for roundup2() and, in fact,
I think DragonFly has already done this.
Regards,
Pedro.
[1] https://people.freebsd.org/~pfg/patches/sys-nitems.diff
[2] For those too lazy to check [1], here is a list of affected files.
M sys/amd64/amd64/amd64_mem.c
M sys/amd64/amd64/machdep.c
M sys/amd64/linux/linux_sysvec.c
M sys/amd64/linux32/linux32_sysvec.c
M sys/arm/amlogic/aml8726/aml8726_clkmsr.c
M sys/arm/arm/db_interface.c
M sys/arm/at91/at91_pmc.c
M sys/arm/mv/armadaxp/armadaxp.c
M sys/arm/ti/cpsw/if_cpsw.c
M sys/arm/xscale/ixp425/ixp425.c
M sys/boot/common/part.c
M sys/boot/efi/loader/bootinfo.c
M sys/boot/mips/beri/boot2/boot2.c
M sys/boot/uboot/common/metadata.c
M sys/cam/ata/ata_da.c
M sys/cam/ata/ata_xpt.c
I would perhaps remove ata_quirk_table_size entirely and replace its
use with nitems() directly. Probably best if that was a separate commit
though from the near-mechanical replacement.
M sys/cam/cam.c
Same here. num_cam_status_entries is only used in one place and I think
directly using nitems() is probably clearer overall.
M sys/cam/scsi/scsi_all.c
Possibly the same here with sense_key_table_size.
M sys/cam/scsi/scsi_cd.c
M sys/cam/scsi/scsi_da.c
M sys/cam/scsi/scsi_sa.c
M sys/cam/scsi/scsi_xpt.c
Same as for ata_quirk_table_size (ironically this used the size in one
place and the expanded form of nitems in another while the ata variant
at least used the size in both places).
M sys/cam/scsi/smp_all.c
M sys/compat/linux/linux_socket.c
M sys/ddb/db_variables.c
M sys/dev/adb/adb_kbd.c
M sys/dev/advansys/adv_isa.c
M sys/dev/advansys/advlib.c
M sys/dev/advansys/adw_pci.c
Same here for num adw_num_pci_devs (only used once)
M sys/dev/advansys/adwlib.c
Probably the same here for adw_num_sync_rates (used twice).
M sys/dev/ae/if_ae.c
Same here for AE_DEVS_COUNT (only used once).
M sys/dev/age/if_age.c
M sys/dev/agp/agp.c
M sys/dev/agp/agp_ali.c
M sys/dev/agp/agp_amd64.c
M sys/dev/aha/aha_isa.c
M sys/dev/aic/aic.c
M sys/dev/aic/aic_cbus.c
Same here for AIC_ISA_NUMPORTS (only used once).
M sys/dev/aic/aic_isa.c
As above.
M sys/dev/ale/if_ale.c
M sys/dev/altera/atse/if_atse.c
M sys/dev/atkbdc/atkbd.c
M sys/dev/atkbdc/atkbdc.c
M sys/dev/atkbdc/psm.c
M sys/dev/bktr/bktr_core.c
M sys/dev/bwi/bwirf.c
M sys/dev/bwn/if_bwn.c
M sys/dev/cardbus/cardbus_cis.c
M sys/dev/digi/digi.c
M sys/dev/digi/digi_isa.c
M sys/dev/dwc/if_dwc.c
M sys/dev/ed/if_ed_hpp.c
M sys/dev/ed/if_ed_isa.c
M sys/dev/ed/if_ed_pci.c
M sys/dev/fb/creator.c
Same here for CREATOR_FB_MAP_SIZE (only used once).
M sys/dev/fb/fb.c
M sys/dev/fb/machfb.c
M sys/dev/fb/vesa.c
M sys/dev/fb/vga.c
M sys/dev/flash/mx25l.c
M sys/dev/hatm/if_hatm.c
M sys/dev/hifn/hifn7751.c
M sys/dev/hwpmc/hwpmc_amd.c
Same here for amd_event_codes_size (only used twice).
M sys/dev/hwpmc/hwpmc_core.c
Same here for niap_events.
M sys/dev/hwpmc/hwpmc_e500.c
e500_event_codes_size isn't even used. It should just be removed.
M sys/dev/hwpmc/hwpmc_mips24k.c
M sys/dev/hwpmc/hwpmc_mips74k.c
M sys/dev/hwpmc/hwpmc_mpc7xxx.c
Same here for mpc7xxx_event_codes_size.
M sys/dev/hwpmc/hwpmc_octeon.c
M sys/dev/hwpmc/hwpmc_uncore.c
Same here for nucp_events.
M sys/dev/hwpmc/hwpmc_xscale.c
Same here for xscale_event_codes_size.
M sys/dev/if_ndis/if_ndis.c
M sys/dev/jme/if_jme.c
M sys/dev/kbd/kbd.c
M sys/dev/le/if_le_isa.c
M sys/dev/le/if_le_lebuffer.c
Same here for NLEMEDIA (only used once).
M sys/dev/le/if_le_ledma.c
M sys/dev/mlx/mlx.c
M sys/dev/mxge/if_mxge.c
M sys/dev/nand/nand_id.c
M sys/dev/ncr/ncr.c
M sys/dev/nctgpio/nctgpio.c
M sys/dev/nfe/if_nfe.c
M sys/dev/patm/if_patm_attach.c
M sys/dev/pccard/pccard_cis_quirks.c
Same here for n_pccard_cis_quirks (only used once).
M sys/dev/rc/rc.c
M sys/dev/re/if_re.c
M sys/dev/rl/if_rl.c
M sys/dev/rndtest/rndtest.c
Same here for RNDTEST_NTESTS (only used once).
M sys/dev/sf/if_sf.c
M sys/dev/sge/if_sge.c
Same here for 'cnt' (only used once).
M sys/dev/siba/siba.c
Same here for 'bound'. I would actually simplify this function
even further so it just does 'return (sd)' instead of 'break'
in the loop body. This means you can remove the '==' check
and just 'return (NULL)' if the loop completes. It also means
that 'bound' is then only used once.
M sys/dev/sio/sio.c
M sys/dev/sound/isa/gusc.c
M sys/dev/sound/pci/emu10kx.c
Same here for 'n_cards' (it is only used once after each assignment).
M sys/dev/speaker/spkr.c
M sys/dev/stge/if_stge.c
M sys/dev/uart/uart_kbd_sun.c
M sys/dev/uart/uart_subr.c
Same here for 'uart_nclasses' (only used once).
M sys/dev/usb/input/ukbd.c
M sys/dev/usb/serial/u3g.c
M sys/dev/usb/serial/uchcom.c
Same here for NUM_DIVIDERS (only used once).
M sys/dev/usb/serial/umcs.c
M sys/dev/usb/serial/uplcom.c
Same here for N_UPLCOM_RATES (only used once).
M sys/dev/vkbd/vkbd.c
M sys/dev/wbwd/wbwd.c
M sys/fs/autofs/autofs.c
M sys/fs/nfs/nfs_commonkrpc.c
M sys/geom/part/g_part_bsd.c
M sys/geom/part/g_part_ebr.c
M sys/geom/part/g_part_ldm.c
M sys/geom/part/g_part_mbr.c
M sys/i386/i386/i686_mem.c
M sys/i386/i386/machdep.c
M sys/i386/ibcs2/ibcs2_sysvec.c
M sys/i386/linux/linux_sysvec.c
M sys/kern/kern_dump.c
M sys/kern/kern_ffclock.c
M sys/kern/kern_jail.c
M sys/kern/kern_ktrace.c
M sys/kern/subr_hash.c
Same here for NPRIMES (only used once).
M sys/kern/subr_witness.c
Same here for blessed_count (only used once).
M sys/kern/sysv_msg.c
M sys/kern/sysv_sem.c
M sys/kern/uipc_usrreq.c
M sys/mips/mips/db_interface.c
M sys/mips/nlm/board.c
M sys/mips/nlm/xlp_machdep.c
Same here for nreg (only used once).
M sys/mips/rmi/dev/nlge/if_nlge.c
Same here for n_gmac_buckers and n_regs (each only used once).
M sys/net/netisr.c
I would do the same here for netisr_dispatch_table_len. It is
used in three loops, but in all three the code would be:
for (i = 0; i < nitems(foo); i++) {
/* use foo[i] */
}
For that idiom, I think using nitems is clearer to the reader vs one of
NFOO, nfoo, foo_size, foo_len, etc. if for no other reason than
consistency. I think it is also more explicit.
M sys/net/rtsock.c
M sys/netgraph/atm/ng_atm.c
M sys/netgraph/bluetooth/socket/ng_btsocket.c
Same here for ng_btsocket_protosw_size (only used once).
M sys/netgraph/ng_gif_demux.c
M sys/netgraph/ng_iface.c
Possibly the same for NUM_FAMILIES in these two files.
