Skip to content

Commit 7b6033e

Browse files
ukleinekjic23
authored andcommitted
iio: adc: ad7173: Fix comparison of channel configs
Checking the binary representation of two structs (of the same type) for equality doesn't have the same semantic as comparing all members for equality. The former might find a difference where the latter doesn't in the presence of padding or when ambiguous types like float or bool are involved. (Floats typically have different representations for single values, like -0.0 vs +0.0, or 0.5 * 2² vs 0.25 * 2³. The type bool has at least 8 bits and the raw values 1 and 2 (probably) both evaluate to true, but memcmp finds a difference.) When searching for a channel that already has the configuration we need, the comparison by member is the one that is needed. Convert the comparison accordingly to compare the members one after another. Also add a static_assert guard to (somewhat) ensure that when struct ad7173_channel_config::config_props is expanded, the comparison is adapted, too. This issue is somewhat theoretic, but using memcmp() on a struct is a bad pattern that is worth fixing. Fixes: 76a1e6a ("iio: adc: ad7173: add AD7173 driver") Signed-off-by: Uwe Kleine-König <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jonathan Cameron <[email protected]>
1 parent 05a5d87 commit 7b6033e

File tree

1 file changed

+21
-4
lines changed

1 file changed

+21
-4
lines changed

drivers/iio/adc/ad7173.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,11 @@ struct ad7173_channel_config {
197197
u8 cfg_slot;
198198
bool live;
199199

200-
/* Following fields are used to compare equality. */
200+
/*
201+
* Following fields are used to compare equality. If you
202+
* make adaptations in it, you most likely also have to adapt
203+
* ad7173_find_live_config(), too.
204+
*/
201205
struct_group(config_props,
202206
bool bipolar;
203207
bool input_buf;
@@ -573,15 +577,28 @@ static struct ad7173_channel_config *
573577
ad7173_find_live_config(struct ad7173_state *st, struct ad7173_channel_config *cfg)
574578
{
575579
struct ad7173_channel_config *cfg_aux;
576-
ptrdiff_t cmp_size;
577580
int i;
578581

579-
cmp_size = sizeof_field(struct ad7173_channel_config, config_props);
582+
/*
583+
* This is just to make sure that the comparison is adapted after
584+
* struct ad7173_channel_config was changed.
585+
*/
586+
static_assert(sizeof_field(struct ad7173_channel_config, config_props) ==
587+
sizeof(struct {
588+
bool bipolar;
589+
bool input_buf;
590+
u8 odr;
591+
u8 ref_sel;
592+
}));
593+
580594
for (i = 0; i < st->num_channels; i++) {
581595
cfg_aux = &st->channels[i].cfg;
582596

583597
if (cfg_aux->live &&
584-
!memcmp(&cfg->config_props, &cfg_aux->config_props, cmp_size))
598+
cfg->bipolar == cfg_aux->bipolar &&
599+
cfg->input_buf == cfg_aux->input_buf &&
600+
cfg->odr == cfg_aux->odr &&
601+
cfg->ref_sel == cfg_aux->ref_sel)
585602
return cfg_aux;
586603
}
587604
return NULL;

0 commit comments

Comments
 (0)