lots of code could be simplified by using ARRAY_SIZE()

lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

  there are numerous places throughout the source tree that apparently
calculate the size of an array using the construct
"sizeof(fubar)/sizeof(fubar[0])". see for yourself:

  $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" *

but we already have, from "include/linux/kernel.h":

  #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

a simple script to make the appropriate cleanup, given the directory
name as an argument:

================== cut ================
#!/bin/sh

DIR=$1

for f in $(grep -Erl "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" ${DIR}) ;
do
  echo "Fixing $f ..."
  perl -pi -e "s|sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)|ARRAY_SIZE\(\1\)|" $f
done
=======================================

  of course, the file must (eventually) include linux/kernel.h but one
would think that applies to the majority of the source tree, no?

  just a thought.

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

On Thu, 14 Dec 2006, Zach Brown wrote:

> >  there are numerous places throughout the source tree that
> > apparently calculate the size of an array using the construct
> > "sizeof(fubar)/sizeof(fubar[0])". see for yourself:

> >  $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" *
>
> Indeed, there seems to be lots of potential clean-up there.
> Including duplicate macros like:
>
> ./drivers/ide/ide-cd.h:#define ARY_LEN(a) ((sizeof(a) / sizeof(a[0])))

not surprisingly, i have a script "arraysize.sh":

============================================================
#!/bin/sh

DIR=$1

# grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" ${DIR}
# grep -Erl "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" ${DIR}

for f in $(grep -Erl "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" ${DIR}) ; do
  echo "ARRAY_SIZE()ing $f ..."
  perl -pi -e "s|sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)|ARRAY_SIZE\(\1\)|" $f
done
===========================================================

  anyone who's interested can run it with a single argument of the
directory to process, eg.:

  $ arraysize.sh fs
  $ arraysize.sh drivers
  $ arraysize.sh .		# entire tree, of course

it's just a first pass, but it seems to produce reasonable code.

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Stefan Richter]

Robert P. J. Day wrote:
> On Thu, 14 Dec 2006, Zach Brown wrote:
...
>> Indeed, there seems to be lots of potential clean-up there.
>> Including duplicate macros like:
>>
>> ./drivers/ide/ide-cd.h:#define ARY_LEN(a) ((sizeof(a) / sizeof(a[0])))
> 
> not surprisingly, i have a script "arraysize.sh":
...

This could also come in the flavor "sizeof(a) / sizeof(*a)".
I haven't checked if there are actual instances.
-- 
Stefan Richter
-=====-=-==- ==-- -====
http://arcgraph.de/sr/

Re: lots of code could be simplified by using ARRAY_SIZE()

[Jan Engelhardt]

>>> Indeed, there seems to be lots of potential clean-up there.
>>> Including duplicate macros like:
>>>
>>> ./drivers/ide/ide-cd.h:#define ARY_LEN(a) ((sizeof(a) / sizeof(a[0])))
>> 
>> not surprisingly, i have a script "arraysize.sh":
>...
>
>This could also come in the flavor "sizeof(a) / sizeof(*a)".
>I haven't checked if there are actual instances.

Even  sizeof a / sizeof *a

may happen.


	-`J'
-- 

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

On Fri, 15 Dec 2006, Jan Engelhardt wrote:

>
> >>> Indeed, there seems to be lots of potential clean-up there.
> >>> Including duplicate macros like:
> >>>
> >>> ./drivers/ide/ide-cd.h:#define ARY_LEN(a) ((sizeof(a) / sizeof(a[0])))
> >>
> >> not surprisingly, i have a script "arraysize.sh":
> >...
> >
> >This could also come in the flavor "sizeof(a) / sizeof(*a)".
> >I haven't checked if there are actual instances.
>
> Even  sizeof a / sizeof *a
>
> may happen.

yes, sadly, there are a number of those as well.  back to the drawing
board.

rday

p.s.  you know, once i nail this script, somebody better apply the end
result.  :-)

Re: lots of code could be simplified by using ARRAY_SIZE()

[Tim Schmielau]

On Fri, 15 Dec 2006, Robert P. J. Day wrote:
> On Fri, 15 Dec 2006, Jan Engelhardt wrote:
> > Even  sizeof a / sizeof *a
> >
> > may happen.
> 
> yes, sadly, there are a number of those as well.  back to the drawing
> board.

It might be interesting to grep for anything that divides two sizeofs and 
eyeball the result for possible mistakes. This would provide some real 
benefit beyond the cosmetical changes.

Tim

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

On Fri, 15 Dec 2006, Tim Schmielau wrote:

> On Fri, 15 Dec 2006, Robert P. J. Day wrote:
> > On Fri, 15 Dec 2006, Jan Engelhardt wrote:
> > > Even  sizeof a / sizeof *a
> > >
> > > may happen.
> >
> > yes, sadly, there are a number of those as well.  back to the drawing
> > board.
>
> It might be interesting to grep for anything that divides two
> sizeofs and eyeball the result for possible mistakes. This would
> provide some real benefit beyond the cosmetical changes.

i did that a while ago and it's amazing the variation that you find
beyond the obvious:

$ grep -Er "sizeof.*/.*sizeof" . | less

...
./net/key/af_key.c:     sa->sadb_sa_len = sizeof(struct sadb_sa)/sizeof(uint64_t);
./net/xfrm/xfrm_policy.c:       int len = sizeof(struct xfrm_selector) / sizeof(u32);
./net/core/flow.c:      const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);
./net/ipv4/netfilter/arp_tables.c:      for (i = 0; i < sizeof(*arp)/sizeof(__u32); i++)
./net/ipv4/af_inet.c:#define INETSW_ARRAY_LEN (sizeof(inetsw_array) / sizeof(struct inet_protosw))
./drivers/net/wireless/ray_cs.c:        .num_standard   = sizeof(ray_handler)/sizeof(iw_handler),

and on and on.  there's no way a cute little perl script is
going to clean up all of *that*.

so what to do?

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Tim Schmielau]

On Sat, 16 Dec 2006, Robert P. J. Day wrote:
> On Fri, 15 Dec 2006, Tim Schmielau wrote:
> >
> > It might be interesting to grep for anything that divides two
> > sizeofs and eyeball the result for possible mistakes. This would
> > provide some real benefit beyond the cosmetical changes.
> 
> i did that a while ago and it's amazing the variation that you find
> beyond the obvious:
> 
> $ grep -Er "sizeof.*/.*sizeof" . | less
> 
> ...
> ./net/key/af_key.c:     sa->sadb_sa_len = sizeof(struct sadb_sa)/sizeof(uint64_t);
> ./net/xfrm/xfrm_policy.c:       int len = sizeof(struct xfrm_selector) / sizeof(u32);
> ./net/core/flow.c:      const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);
> ./net/ipv4/netfilter/arp_tables.c:      for (i = 0; i < sizeof(*arp)/sizeof(__u32); i++)
> ./net/ipv4/af_inet.c:#define INETSW_ARRAY_LEN (sizeof(inetsw_array) / sizeof(struct inet_protosw))
> ./drivers/net/wireless/ray_cs.c:        .num_standard   = sizeof(ray_handler)/sizeof(iw_handler),
> 

Of the above, af_inet.c and ray_cs.c seem to be good candidates for 
ARRAY_SIZE. You might even remove the INETSW_ARRAY_LEN #define in 
af_inet.c altogether, since ARRAY_SIZE(inetsw_array) is quite readable.

xfrm_policy.c, flow.c and arp_tables.c seem to be reasonably readable 
trickery that can be left as-is.

>From a first glance, af_key.c is ok but might profit from a comment 
in include/linux/pfkeyv2.h saying that sadb_msg_len is measured in 64-bit 
words per RFC 2367. Though documenting the structs in pfkeyv2.h would be
quite a bit different from what you initially intended...

So, if you have some time to spend on this, manual inspection would 
probably be the most useful thing, since any automatic sed tricks will 
only replace what a human ready would easily understand as well.

If you manually generate cleanup patches, it would be very good to check 
that compilation with allyesconfig generates identical code before and 
after before feeding them through the respective maintainers.

If you want to find genuine bugs by this, it might be more worthwhile to 
start with drivers/, as davem is just too clever to make any mistakes. 

Not that I want to make you spend you time on this. It's just beause you 
asked.

Tim

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

(i'm not *trying* to belabour this issue ... i am merely succeeding)

On Sat, 16 Dec 2006, Tim Schmielau wrote:

> On Sat, 16 Dec 2006, Robert P. J. Day wrote:
...
> > ... it's amazing the variation that you find beyond the obvious:
> >
> > $ grep -Er "sizeof.*/.*sizeof" . | less
> >
> > ...
> > ./net/key/af_key.c:     sa->sadb_sa_len = sizeof(struct sadb_sa)/sizeof(uint64_t);
> > ./net/xfrm/xfrm_policy.c:       int len = sizeof(struct xfrm_selector) / sizeof(u32);
> > ./net/core/flow.c:      const int n_elem = sizeof(struct flowi) / sizeof(flow_compare_t);
> > ./net/ipv4/netfilter/arp_tables.c:      for (i = 0; i < sizeof(*arp)/sizeof(__u32); i++)
> > ./net/ipv4/af_inet.c:#define INETSW_ARRAY_LEN (sizeof(inetsw_array) / sizeof(struct inet_protosw))
> > ./drivers/net/wireless/ray_cs.c:        .num_standard   = sizeof(ray_handler)/sizeof(iw_handler),
> >
>
> Of the above, af_inet.c and ray_cs.c seem to be good candidates for
> ARRAY_SIZE. You might even remove the INETSW_ARRAY_LEN #define in
> af_inet.c altogether, since ARRAY_SIZE(inetsw_array) is quite
> readable.

note that the above examples i listed were just a *few* of the
examples that didn't match the most common variants:

  sizeof(fubar) / sizeof(fubar[0])
  sizeof(fubar) / sizeof(*fubar)

i just did that to show that, even if i can run a script to handle the
most common variants, there would be lots of manual cleanup left.

> From a first glance, af_key.c is ok but might profit from a comment
> in include/linux/pfkeyv2.h saying that sadb_msg_len is measured in
> 64-bit words per RFC 2367. Though documenting the structs in
> pfkeyv2.h would be quite a bit different from what you initially
> intended...

in fact, i just emailed a short CodingStyle note to randy dunlap
(since he seemed to be heavily into the coding style stuff),
suggesting that a short note be added strongly recommending that one
should use ARRAY_SIZE wherever possible and, if not possible, a
comment should be added explaining why not, if it seems to be useful.

> So, if you have some time to spend on this, manual inspection would
> probably be the most useful thing, since any automatic sed tricks
> will only replace what a human ready would easily understand as
> well.

true enough, but if the most common variants can be handled
automatically, then the remainder would stand out more obviously and
could be manually handled from there.

> If you manually generate cleanup patches, it would be very good to
> check that compilation with allyesconfig generates identical code
> before and after before feeding them through the respective
> maintainers.

i'm actually in the process of trying that as we speak, at least with
the automatic cleanup.  there's no way i'm going to try to get into
manual cleanup with all of those weird variants.  life's too short for
that.  :-)

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2071 bytes --]


  so here's the end result of my experiment to replace unnecessary
code snippets with an invocation of the ARRAY_SIZE() macro from
include/linux/kernel.h.  i've attached the script that i ran on the
entire tree, then (after adding al viro's connector patch), did:

  $ make allyesconfig	# for the stress factor
  $ make

to see what would happen.

  amazingly, the compile worked all the way down to:

  AS      arch/i386/boot/bootsect.o
  LD      arch/i386/boot/bootsect
  AS      arch/i386/boot/setup.o
  LD      arch/i386/boot/setup
  AS      arch/i386/boot/compressed/head.o
  CC      arch/i386/boot/compressed/misc.o
  OBJCOPY arch/i386/boot/compressed/vmlinux.bin
  HOSTCC  arch/i386/boot/compressed/relocs
arch/i386/boot/compressed/relocs.c: In function 'sym_type':
arch/i386/boot/compressed/relocs.c:72: warning: implicit declaration of function 'ARRAY_SIZE'
/tmp/ccRTpFxM.o: In function `main':
relocs.c:(.text+0xb13): undefined reference to `ARRAY_SIZE'
relocs.c:(.text+0xddb): undefined reference to `ARRAY_SIZE'
relocs.c:(.text+0xe10): undefined reference to `ARRAY_SIZE'
relocs.c:(.text+0xe2b): undefined reference to `ARRAY_SIZE'
collect2: ld returned 1 exit status
make[2]: *** [arch/i386/boot/compressed/relocs] Error 1
make[1]: *** [arch/i386/boot/compressed/vmlinux] Error 2
make: *** [bzImage] Error 2

  not surprisingly, the diff is fairly sizable (2408 lines), and
that's *way* outside of my comfort zone in terms of what i would
submit as a patch.  others higher up the food chain can decide if they
want to do anything with this.  (for the information value, i also
attached the "diffstat" output.)

  at this point, i think it's someone else's call.

rday

p.s.  clearly, this didn't even hit all of the possible
transformations, such as the ones based on typedefs, or even the ones
that are broken over two lines.  you can see from the script that i
just went after the low-hanging fruit.

p.p.s.  i didn't bother fixing relocs.c yet to see if the build would
actually finish.  i thought i'd better stop here and wait to hear what
others think.

[-- Attachment #2: Type: APPLICATION/x-sh, Size: 1472 bytes --]

[-- Attachment #3: Type: TEXT/PLAIN, Size: 7293 bytes --]

 Documentation/hpet.txt                            |    2 
 arch/arm/kernel/dma-isa.c                         |    2 
 arch/arm/kernel/ecard.c                           |    2 
 arch/arm26/kernel/ecard.c                         |    2 
 arch/avr32/kernel/setup.c                         |    2 
 arch/cris/arch-v10/drivers/axisflashmap.c         |    2 
 arch/cris/arch-v10/kernel/setup.c                 |    4 -
 arch/cris/mm/tlb.c                                |    2 
 arch/i386/boot/compressed/relocs.c                |    8 +--
 arch/m68k/amiga/amisound.c                        |    2 
 arch/m68k/kernel/ptrace.c                         |    4 -
 arch/m68k/kernel/traps.c                          |    2 
 arch/m68knommu/kernel/ptrace.c                    |    4 -
 arch/m68knommu/kernel/traps.c                     |    2 
 arch/mips/arc/identify.c                          |    2 
 arch/mips/jmr3927/rbhma3100/setup.c               |    2 
 arch/mips/mips-boards/atlas/atlas_int.c           |    4 -
 arch/mips/pci/fixup-vr4133.c                      |    2 
 arch/powerpc/lib/rheap.c                          |    2 
 arch/powerpc/xmon/ppc-opc.c                       |    4 -
 arch/powerpc/xmon/spu-opc.c                       |    2 
 arch/ppc/lib/rheap.c                              |    2 
 arch/ppc/syslib/m8xx_setup.c                      |    2 
 arch/ppc/xmon/ppc-opc.c                           |    4 -
 arch/um/kernel/tt/ptproxy/ptrace.c                |   12 ++--
 arch/v850/kernel/anna.c                           |    2 
 arch/v850/kernel/as85ep1.c                        |    2 
 arch/v850/kernel/fpga85e2c.c                      |    2 
 arch/v850/kernel/gbus_int.c                       |    4 -
 arch/v850/kernel/ma.c                             |    2 
 arch/v850/kernel/me2.c                            |    2 
 arch/v850/kernel/rte_cb.c                         |    2 
 arch/v850/kernel/rte_mb_a_pci.c                   |    2 
 arch/v850/kernel/rte_me2_cb.c                     |    2 
 arch/v850/kernel/teg.c                            |    2 
 arch/x86_64/kernel/smpboot.c                      |    2 
 drivers/acorn/block/fd1772.c                      |    2 
 drivers/acpi/toshiba_acpi.c                       |    2 
 drivers/atm/he.c                                  |    2 
 drivers/atm/idt77252.c                            |    8 +--
 drivers/block/acsi.c                              |    4 -
 drivers/block/acsi_slm.c                          |    4 -
 drivers/char/drm/drm_proc.c                       |    2 
 drivers/char/synclink_gt.c                        |    2 
 drivers/connector/connector.c                     |    3 -
 drivers/ide/ide-cd.h                              |    2 
 drivers/infiniband/core/device.c                  |    2 
 drivers/isdn/capi/capi.c                          |    4 -
 drivers/isdn/capi/capidrv.c                       |    4 -
 drivers/isdn/hardware/eicon/debug.c               |   30 ++++++------
 drivers/isdn/hardware/eicon/message.c             |   22 ++++----
 drivers/kvm/kvm_svm.h                             |    2 
 drivers/kvm/svm.c                                 |    4 -
 drivers/kvm/vmx.c                                 |    4 -
 drivers/macintosh/apm_emu.c                       |    2 
 drivers/md/md.c                                   |    4 -
 drivers/media/dvb/bt8xx/dvb-bt8xx.c               |    2 
 drivers/media/dvb/frontends/cx24110.c             |    4 -
 drivers/media/dvb/frontends/cx24123.c             |    6 +-
 drivers/media/video/cpia2/cpia2_v4l.c             |    8 +--
 drivers/media/video/ov7670.c                      |    6 +-
 drivers/media/video/pvrusb2/pvrusb2-audio.c       |    4 -
 drivers/media/video/pvrusb2/pvrusb2-ctrl.c        |    2 
 drivers/media/video/pvrusb2/pvrusb2-cx2584x-v4l.c |    4 -
 drivers/media/video/pvrusb2/pvrusb2-debugifc.c    |    4 -
 drivers/media/video/pvrusb2/pvrusb2-eeprom.c      |    2 
 drivers/media/video/pvrusb2/pvrusb2-encoder.c     |   16 +++---
 drivers/media/video/pvrusb2/pvrusb2-hdw.c         |   16 +++---
 drivers/media/video/pvrusb2/pvrusb2-i2c-core.c    |    2 
 drivers/media/video/pvrusb2/pvrusb2-std.c         |   14 ++---
 drivers/media/video/pvrusb2/pvrusb2-sysfs.c       |    2 
 drivers/media/video/pvrusb2/pvrusb2-video-v4l.c   |    4 -
 drivers/media/video/pvrusb2/pvrusb2-wm8775.c      |    4 -
 drivers/media/video/tveeprom.c                    |    6 +-
 drivers/media/video/tvp5150.c                     |    2 
 drivers/media/video/usbvideo/quickcam_messenger.c |    2 
 drivers/message/fusion/mptbase.c                  |    2 
 drivers/mmc/au1xmmc.c                             |    2 
 drivers/net/apne.c                                |    2 
 drivers/net/arm/am79c961a.c                       |    2 
 drivers/net/atarilance.c                          |    2 
 drivers/net/cs89x0.c                              |    6 +-
 drivers/net/e1000/e1000_ethtool.c                 |    2 
 drivers/net/fec_8xx/fec_mii.c                     |    4 -
 drivers/net/ibm_emac/ibm_emac_debug.c             |    8 +--
 drivers/net/irda/actisys-sir.c                    |    2 
 drivers/net/ixgb/ixgb_param.c                     |    2 
 drivers/net/ne-h8300.c                            |    2 
 drivers/net/ne.c                                  |    2 
 drivers/net/ne2.c                                 |    2 
 drivers/net/ne2k-pci.c                            |    2 
 drivers/net/netxen/netxen_nic_hw.c                |    2 
 drivers/net/pcmcia/axnet_cs.c                     |    2 
 drivers/net/pcmcia/pcnet_cs.c                     |    2 
 drivers/net/sk98lin/skgemib.c                     |    2 
 drivers/net/sk98lin/skgesirq.c                    |    2 
 drivers/net/skfp/smt.c                            |    2 
 drivers/net/skfp/srf.c                            |    4 -
 drivers/net/wireless/airo.c                       |    4 -
 drivers/net/wireless/hostap/hostap.h              |    2 
 drivers/net/wireless/ipw2100.c                    |   12 ++--
 drivers/net/wireless/prism54/oid_mgt.c            |    2 
 drivers/net/wireless/wavelan.p.h                  |    2 
 drivers/net/zorro8390.c                           |    2 
 drivers/s390/cio/device_id.c                      |    2 
 drivers/serial/68328serial.c                      |    6 +-
 drivers/serial/mcfserial.c                        |    2 
 drivers/w1/slaves/w1_therm.c                      |    6 +-
 fs/reiserfs/do_balan.c                            |    4 -
 include/asm-parisc/mmzone.h                       |    2 
 include/linux/netfilter/xt_sctp.h                 |    2 
 include/net/ip_vs.h                               |    2 
 include/video/sgivw.h                             |    2 
 kernel/rcutorture.c                               |    2 
 net/atm/proc.c                                    |    2 
 sound/oss/au1550_ac97.c                           |    4 -
 sound/oss/es1371.c                                |    2 
 sound/oss/nec_vrc5477.c                           |    4 -
 sound/oss/swarm_cs4297a.c                         |    2 
 119 files changed, 223 insertions(+), 224 deletions(-)

Re: lots of code could be simplified by using ARRAY_SIZE()

[Randy Dunlap]

On Sun, 17 Dec 2006 13:13:59 -0500 (EST) Robert P. J. Day wrote:

> 
>   so here's the end result of my experiment to replace unnecessary
> code snippets with an invocation of the ARRAY_SIZE() macro from
> include/linux/kernel.h.  i've attached the script that i ran on the
> entire tree, then (after adding al viro's connector patch), did:
> 
>   $ make allyesconfig	# for the stress factor
>   $ make
> 
> to see what would happen.
> 
>   amazingly, the compile worked all the way down to:
> 
>   AS      arch/i386/boot/bootsect.o
>   LD      arch/i386/boot/bootsect
>   AS      arch/i386/boot/setup.o
>   LD      arch/i386/boot/setup
>   AS      arch/i386/boot/compressed/head.o
>   CC      arch/i386/boot/compressed/misc.o
>   OBJCOPY arch/i386/boot/compressed/vmlinux.bin
>   HOSTCC  arch/i386/boot/compressed/relocs
> arch/i386/boot/compressed/relocs.c: In function 'sym_type':
> arch/i386/boot/compressed/relocs.c:72: warning: implicit declaration of function 'ARRAY_SIZE'

That's a userspace program and shouldn't use kernel.h.

> /tmp/ccRTpFxM.o: In function `main':
> relocs.c:(.text+0xb13): undefined reference to `ARRAY_SIZE'
> relocs.c:(.text+0xddb): undefined reference to `ARRAY_SIZE'
> relocs.c:(.text+0xe10): undefined reference to `ARRAY_SIZE'
> relocs.c:(.text+0xe2b): undefined reference to `ARRAY_SIZE'
> collect2: ld returned 1 exit status
> make[2]: *** [arch/i386/boot/compressed/relocs] Error 1
> make[1]: *** [arch/i386/boot/compressed/vmlinux] Error 2
> make: *** [bzImage] Error 2

---
~Randy

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

On Sun, 17 Dec 2006, Randy Dunlap wrote:

> On Sun, 17 Dec 2006 13:13:59 -0500 (EST) Robert P. J. Day wrote:
>
> >
> >   so here's the end result of my experiment to replace unnecessary
> > code snippets with an invocation of the ARRAY_SIZE() macro from
> > include/linux/kernel.h.  i've attached the script that i ran on the
> > entire tree, then (after adding al viro's connector patch), did:
> >
> >   $ make allyesconfig	# for the stress factor
> >   $ make
> >
> > to see what would happen.
> >
> >   amazingly, the compile worked all the way down to:
> >
> >   AS      arch/i386/boot/bootsect.o
> >   LD      arch/i386/boot/bootsect
> >   AS      arch/i386/boot/setup.o
> >   LD      arch/i386/boot/setup
> >   AS      arch/i386/boot/compressed/head.o
> >   CC      arch/i386/boot/compressed/misc.o
> >   OBJCOPY arch/i386/boot/compressed/vmlinux.bin
> >   HOSTCC  arch/i386/boot/compressed/relocs
> > arch/i386/boot/compressed/relocs.c: In function 'sym_type':
> > arch/i386/boot/compressed/relocs.c:72: warning: implicit declaration of function 'ARRAY_SIZE'
>
> That's a userspace program and shouldn't use kernel.h.

ah, quite right, my bad.  eggnog hangover.

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Miguel Ojeda]

On 12/13/06, Robert P. J. Day <rpjday@mindspring.com> wrote:
>
>   there are numerous places throughout the source tree that apparently
> calculate the size of an array using the construct
> "sizeof(fubar)/sizeof(fubar[0])". see for yourself:
>
>   $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" *
>
> but we already have, from "include/linux/kernel.h":
>
>   #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

Maybe *(x) instead of (x)[0]?

-- 
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

On Fri, 15 Dec 2006, Miguel Ojeda wrote:

> On 12/13/06, Robert P. J. Day <rpjday@mindspring.com> wrote:
> >
> >   there are numerous places throughout the source tree that apparently
> > calculate the size of an array using the construct
> > "sizeof(fubar)/sizeof(fubar[0])". see for yourself:
> >
> >   $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" *
> >
> > but we already have, from "include/linux/kernel.h":
> >
> >   #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
> Maybe *(x) instead of (x)[0]?

yeah, there's a bunch of that, too:

  $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\*\1\)" .

easy enough to catch using the same technique.

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Pavel Machek]

Hi!

>   there are numerous places throughout the source tree that apparently
> calculate the size of an array using the construct
> "sizeof(fubar)/sizeof(fubar[0])". see for yourself:
> 
>   $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" *
> 
> but we already have, from "include/linux/kernel.h":
> 
>   #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

Hmmm. quite misleading name :-(. ARRAY_LEN would be better.

-- 
Thanks for all the (sleeping) penguins.

Re: lots of code could be simplified by using ARRAY_SIZE()

[Robert P. J. Day]

On Sat, 16 Dec 2006, Pavel Machek wrote:

> Hi!
>
> >   there are numerous places throughout the source tree that apparently
> > calculate the size of an array using the construct
> > "sizeof(fubar)/sizeof(fubar[0])". see for yourself:
> >
> >   $ grep -Er "sizeof\((.*)\) ?/ ?sizeof\(\1\[0\]\)" *
> >
> > but we already have, from "include/linux/kernel.h":
> >
> >   #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
> Hmmm. quite misleading name :-(. ARRAY_LEN would be better.

i suspect it's *way* too late to make that kind of change, given that
"ARRAY_SIZE" is firmly ensconced in countless places in the source
tree and that would be a major, disruptive change.

even *i* wouldn't try to promote that idea.  :-)

rday

Re: lots of code could be simplified by using ARRAY_SIZE()

[Jan Engelhardt]

On Dec 16 2006 08:09, Robert P. J. Day wrote:
>On Sat, 16 Dec 2006, Pavel Machek wrote:
>> > but we already have, from "include/linux/kernel.h":
>> >
>> >   #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>
>> Hmmm. quite misleading name :-(. ARRAY_LEN would be better.
>
>i suspect it's *way* too late to make that kind of change, given that
>"ARRAY_SIZE" is firmly ensconced in countless places in the source
>tree and that would be a major, disruptive change.
>
>even *i* wouldn't try to promote that idea.  :-)

You know, you could always make it compat for a while, but that requires
approval from Linus I suppose /* heh, heh */

I don't even know if this will compile everywhere,
but I hope you can figure out the idea...

#define ARRAY_SIZE(x) (print_warning(), sizeof(x) / sizeof(*x))
#define ARRAY_LEN(x)  (sizeof(x) / sizeof(*x))
extern ...
void print_warning(void) {
    printk("Don't use ARRAY_SIZE anymore, it will go away\n");
}


	-`J'
-- 

Re: lots of code could be simplified by using ARRAY_SIZE()

[Stefan Richter]

Jan Engelhardt wrote:
> On Dec 16 2006 08:09, Robert P. J. Day wrote:
>> On Sat, 16 Dec 2006, Pavel Machek wrote:
>>>> but we already have, from "include/linux/kernel.h":
>>>>
>>>>   #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>> Hmmm. quite misleading name :-(. ARRAY_LEN would be better.

"Size", "length", "width", "depth" is all the same in that one needs to
know the unit of measurement. The unit of measurement of ARRAY_SIZE is
one array member. This makes it useful as a bound for [ ] pointer
arithmetic which uses the same unit.

If you want to look at it from a slightly higher level of abstraction
and want to avoid the ambiguity WRT units of measurement (C programs
most often use Byte as unit for data size), consider the unitless
(cardinal) ARRAY_INDEX_BOUND or ARRAY_CARDINALITY. (In a language which
starts array indexes at 1 instead of 0, it could also be called
ARRAY_HIINDEX.)

But fortunately...

>> i suspect it's *way* too late to make that kind of change, given that
>> "ARRAY_SIZE" is firmly ensconced in countless places in the source
>> tree and that would be a major, disruptive change.
>>
>> even *i* wouldn't try to promote that idea.  :-)
> 
> You know, you could always make it compat for a while, but that requires
> approval from Linus I suppose /* heh, heh */
> 
> I don't even know if this will compile everywhere,
> but I hope you can figure out the idea...
> 
> #define ARRAY_SIZE(x) (print_warning(), sizeof(x) / sizeof(*x))
> #define ARRAY_LEN(x)  (sizeof(x) / sizeof(*x))
> extern ...
> void print_warning(void) {
>     printk("Don't use ARRAY_SIZE anymore, it will go away\n");
> }

...those who know that the ARRAY_SIZE macro is available also know what
it means and how to use it. Therefore there is no need to rename this macro.
-- 
Stefan Richter
-=====-=-==- ==-- =---=
http://arcgraph.de/sr/