public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* rfc: ARRAY_AND_SIZE
@ 2013-08-09  8:07 walter harms
  2013-08-09  8:15 ` Julia Lawall
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: walter harms @ 2013-08-09  8:07 UTC (permalink / raw)
  To: kernel-janitors

hi list,
while looking at Julias nice patch. i notice the ARRAY_AND_SIZE macro
it looks like that:

#define ARRAY_AND_SIZE(x)       (x), ARRAY_SIZE(x)

I see problem: hiding an argument in function calls is bad.
(personally i hate hiding arguments in function calls therefore i am not objective)

the next problem is that it is defined 6 times in different locations.
(see:http://lxr.free-electrons.com/ident?i=ARRAY_AND_SIZE)

But so far it is used in the ARM tree mostly.

Is there any policy on that ?

re,
 wh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rfc: ARRAY_AND_SIZE
  2013-08-09  8:07 rfc: ARRAY_AND_SIZE walter harms
@ 2013-08-09  8:15 ` Julia Lawall
  2013-08-09  9:10 ` Dan Carpenter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2013-08-09  8:15 UTC (permalink / raw)
  To: kernel-janitors

On Fri, 9 Aug 2013, walter harms wrote:

> hi list,
> while looking at Julias nice patch. i notice the ARRAY_AND_SIZE macro
> it looks like that:
>
> #define ARRAY_AND_SIZE(x)       (x), ARRAY_SIZE(x)
>
> I see problem: hiding an argument in function calls is bad.
> (personally i hate hiding arguments in function calls therefore i am not objective)
>
> the next problem is that it is defined 6 times in different locations.
> (see:http://lxr.free-electrons.com/ident?i=ARRAY_AND_SIZE)
>
> But so far it is used in the ARM tree mostly.
>
> Is there any policy on that ?

It is indeed quite disconcerting.  For example, I found the following:

arch/arm/mach-pxa/stargate2.c

991         i2c_register_board_info(0, ARRAY_AND_SIZE(stargate2_i2c_board_info));
992         i2c_register_board_info(1, stargate2_pwr_i2c_board_info,
993                                 ARRAY_SIZE(stargate2_pwr_i2c_board_info));

On the other hand, with ARRAY_AND_SIZE, one is sure that the two arguments
are the same...

julia

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rfc: ARRAY_AND_SIZE
  2013-08-09  8:07 rfc: ARRAY_AND_SIZE walter harms
  2013-08-09  8:15 ` Julia Lawall
@ 2013-08-09  9:10 ` Dan Carpenter
  2013-08-09  9:23 ` Julia Lawall
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-08-09  9:10 UTC (permalink / raw)
  To: kernel-janitors

It's mostly used like this:

static struct ltq_pmx_func falcon_funcs[] = {
	{"rst",         ARRAY_AND_SIZE(ltq_rst_grps)},
	{"ntr",         ARRAY_AND_SIZE(ltq_ntr_grps)},
	{"mdio",        ARRAY_AND_SIZE(ltq_mdio_grps)},
	{"led",         ARRAY_AND_SIZE(ltq_bled_grps)},
	{"asc",         ARRAY_AND_SIZE(ltq_asc_grps)},
	{"spi",         ARRAY_AND_SIZE(ltq_spi_grps)},
	{"i2c",         ARRAY_AND_SIZE(ltq_i2c_grps)},
	{"jtag",        ARRAY_AND_SIZE(ltq_jtag_grps)},
	{"slic",        ARRAY_AND_SIZE(ltq_slic_grps)},
	{"pcm",         ARRAY_AND_SIZE(ltq_pcm_grps)},
};

That's fine.

There are a few times when it's used as function parameters, which
looks like garbage.  If I were the maintainer, I would change it,
but I'm not so it's not worth arguing with maintainers about
something they did deliberately.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rfc: ARRAY_AND_SIZE
  2013-08-09  8:07 rfc: ARRAY_AND_SIZE walter harms
  2013-08-09  8:15 ` Julia Lawall
  2013-08-09  9:10 ` Dan Carpenter
@ 2013-08-09  9:23 ` Julia Lawall
  2013-08-09  9:36 ` Dan Carpenter
  2013-08-09 12:32 ` walter harms
  4 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2013-08-09  9:23 UTC (permalink / raw)
  To: kernel-janitors

On Fri, 9 Aug 2013, Dan Carpenter wrote:

> It's mostly used like this:
>
> static struct ltq_pmx_func falcon_funcs[] = {
> 	{"rst",         ARRAY_AND_SIZE(ltq_rst_grps)},
> 	{"ntr",         ARRAY_AND_SIZE(ltq_ntr_grps)},
> 	{"mdio",        ARRAY_AND_SIZE(ltq_mdio_grps)},
> 	{"led",         ARRAY_AND_SIZE(ltq_bled_grps)},
> 	{"asc",         ARRAY_AND_SIZE(ltq_asc_grps)},
> 	{"spi",         ARRAY_AND_SIZE(ltq_spi_grps)},
> 	{"i2c",         ARRAY_AND_SIZE(ltq_i2c_grps)},
> 	{"jtag",        ARRAY_AND_SIZE(ltq_jtag_grps)},
> 	{"slic",        ARRAY_AND_SIZE(ltq_slic_grps)},
> 	{"pcm",         ARRAY_AND_SIZE(ltq_pcm_grps)},
> };
>
> That's fine.
>
> There are a few times when it's used as function parameters, which
> looks like garbage.  If I were the maintainer, I would change it,
> but I'm not so it's not worth arguing with maintainers about
> something they did deliberately.

Actually, it is used 226 times as function arguments, in 77 files.  It is
used 39 times in a structure declaration.

julia

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rfc: ARRAY_AND_SIZE
  2013-08-09  8:07 rfc: ARRAY_AND_SIZE walter harms
                   ` (2 preceding siblings ...)
  2013-08-09  9:23 ` Julia Lawall
@ 2013-08-09  9:36 ` Dan Carpenter
  2013-08-09 12:32 ` walter harms
  4 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2013-08-09  9:36 UTC (permalink / raw)
  To: kernel-janitors

On Fri, Aug 09, 2013 at 11:23:06AM +0200, Julia Lawall wrote:

> Actually, it is used 226 times as function arguments, in 77 files.  It is
> used 39 times in a structure declaration.

Ah...  I was using cscope and so I missed ARM uses that weren't in
my config.

That's ugly, yes.  But I still think the maintainers aren't going to
appreciate patches for this since they did it deliberately.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: rfc: ARRAY_AND_SIZE
  2013-08-09  8:07 rfc: ARRAY_AND_SIZE walter harms
                   ` (3 preceding siblings ...)
  2013-08-09  9:36 ` Dan Carpenter
@ 2013-08-09 12:32 ` walter harms
  4 siblings, 0 replies; 6+ messages in thread
From: walter harms @ 2013-08-09 12:32 UTC (permalink / raw)
  To: kernel-janitors



Am 09.08.2013 11:36, schrieb Dan Carpenter:
> On Fri, Aug 09, 2013 at 11:23:06AM +0200, Julia Lawall wrote:
> 
>> Actually, it is used 226 times as function arguments, in 77 files.  It is
>> used 39 times in a structure declaration.
> 
> Ah...  I was using cscope and so I missed ARM uses that weren't in
> my config.
> 
> That's ugly, yes.  But I still think the maintainers aren't going to
> appreciate patches for this since they did it deliberately.
> 


This is the point why i started a RFC. There are 6 definitions
what means 5 have to go. NTL i would question the wisdom of having
that thing in the first had.

re,
 wh

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-08-09 12:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-09  8:07 rfc: ARRAY_AND_SIZE walter harms
2013-08-09  8:15 ` Julia Lawall
2013-08-09  9:10 ` Dan Carpenter
2013-08-09  9:23 ` Julia Lawall
2013-08-09  9:36 ` Dan Carpenter
2013-08-09 12:32 ` walter harms

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox