]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
iio: adc: ad7173: Fix comparison of channel configs
authorUwe Kleine-König <u.kleine-koenig@baylibre.com>
Mon, 3 Mar 2025 11:47:02 +0000 (12:47 +0100)
committerJonathan Cameron <Jonathan.Cameron@huawei.com>
Tue, 11 Mar 2025 19:09:16 +0000 (19:09 +0000)
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: 76a1e6a42802 ("iio: adc: ad7173: add AD7173 driver")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Link: https://patch.msgid.link/20250303114659.1672695-14-u.kleine-koenig@baylibre.com
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
drivers/iio/adc/ad7173.c

index 9ac8ea3cb4997847ecc7b8c1361f499b0facbb2f..69de5886474ce2f700bf277ce707b15637113564 100644 (file)
@@ -197,7 +197,11 @@ struct ad7173_channel_config {
        u8 cfg_slot;
        bool live;
 
-       /* Following fields are used to compare equality. */
+       /*
+        * Following fields are used to compare equality. If you
+        * make adaptations in it, you most likely also have to adapt
+        * ad7173_find_live_config(), too.
+        */
        struct_group(config_props,
                bool bipolar;
                bool input_buf;
@@ -573,15 +577,28 @@ static struct ad7173_channel_config *
 ad7173_find_live_config(struct ad7173_state *st, struct ad7173_channel_config *cfg)
 {
        struct ad7173_channel_config *cfg_aux;
-       ptrdiff_t cmp_size;
        int i;
 
-       cmp_size = sizeof_field(struct ad7173_channel_config, config_props);
+       /*
+        * This is just to make sure that the comparison is adapted after
+        * struct ad7173_channel_config was changed.
+        */
+       static_assert(sizeof_field(struct ad7173_channel_config, config_props) ==
+                     sizeof(struct {
+                                    bool bipolar;
+                                    bool input_buf;
+                                    u8 odr;
+                                    u8 ref_sel;
+                            }));
+
        for (i = 0; i < st->num_channels; i++) {
                cfg_aux = &st->channels[i].cfg;
 
                if (cfg_aux->live &&
-                   !memcmp(&cfg->config_props, &cfg_aux->config_props, cmp_size))
+                   cfg->bipolar == cfg_aux->bipolar &&
+                   cfg->input_buf == cfg_aux->input_buf &&
+                   cfg->odr == cfg_aux->odr &&
+                   cfg->ref_sel == cfg_aux->ref_sel)
                        return cfg_aux;
        }
        return NULL;