Skip to contentSkip to conversation

rfc: ARRAY_AND_SIZE

DISCUSSION walter harms 3 participants 6 messages Aug 9, 2013 Kernel janitors

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 9 lines · walter harms

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)

Quoted 6 lines

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

#2 lore raw
In reply to walter harms

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

#3 lore raw
In reply to walter harms
Quoted 15 lines · Dan Carpenter

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)},
};
Quoted 6 lines

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

#4 lore raw
In reply to walter harms
Quoted 2 lines · Julia Lawall

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

#5 lore raw
In reply to walter harms

Am 09.08.2013 11:36, schrieb Dan Carpenter:

Quoted 11 lines · Julia Lawall
Quoted 3 lines

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