bi_pin_mask_with_names assumes order

[ecrips]

The SDK provides 3 functions of the form bi_2pins_with_names which allow specifying multiple pins and their names. However, because they are implemented in terms of bi_pin_mask_with_names which takes a single bitmask, there is an (undocumented!) assumption that the pins are passed in ascending numeric order.

For example:

bi_decl_if_func_used(bi_2pins_with_names( 5, "Pin 5", 4, "Pin 4" ))

produces the following output from picotool:

Fixed Pin Information
 4:   Pin 5
 5:   Pin 4

Swapping the order of the argument fixes it to behave as expected. Obviously the GPIO numbers might not be hardcoded like that but may come from e.g. a configuration option.

A patch like the following at least triggers a build error, even better if someone can work out a way to rearrange the order automatically.

diff --git a/src/common/pico_binary_info/include/pico/binary_info/code.h b/src/common/pico_binary_info/include/pico/binary_info/code.h
index e87a2cd..7eb75d9 100644
--- a/src/common/pico_binary_info/include/pico/binary_info/code.h
+++ b/src/common/pico_binary_info/include/pico/binary_info/code.h
@@ -134,8 +134,13 @@ static const struct _binary_info_named_group __bi_lineno_var_name = { \
 // names are sperated by | ... i.e. "name1|name2|name3"
 #define bi_pin_mask_with_names(pmask, label)          __bi_pins_with_name((pmask), (label))
 #define bi_1pin_with_name(p0, name)                   bi_pin_mask_with_name(1u << (p0), name)
-#define bi_2pins_with_names(p0, name0, p1, name1)     bi_pin_mask_with_names((1u << (p0)) | (1u << (p1)), name0 "|" name1)
-#define bi_3pins_with_names(p0, name0, p1, name1, p2, name2)  bi_pin_mask_with_names((1u << (p0)) | (1u << (p1)) | (1u << (p2)), name0 "|" name1 "|" name2)
-#define bi_4pins_with_names(p0, name0, p1, name1, p2, name2, p3, name3)  bi_pin_mask_with_names((1u << (p0)) | (1u << (p1)) | (1u << (p2)) | (1u << (p3)), name0 "|" name1 "|" name2 "|" name3)
+#define bi_combine_pins(p0, p1)                       ({static_assert(p0 < p1, "Pin numbers must be in order"); p0 | p1; })
+#define bi_2pins_with_names(p0, name0, p1, name1)     bi_pin_mask_with_names(bi_combine_pins(1u << (p0), 1u << (p1)), name0 "|" name1)
+#define bi_3pins_with_names(p0, name0, p1, name1, p2, name2)  bi_pin_mask_with_names(bi_combine_pins(1u << (p0),                                          \
+                                                                                                     bi_combine_pins(1u << (p1), 1u << (p2))),            \
+                                                                                                     name0 "|" name1 "|" name2)
+#define bi_4pins_with_names(p0, name0, p1, name1, p2, name2, p3, name3)  bi_pin_mask_with_names(bi_combine_pins(bi_combine_pins(1u << (p0), 1u << (p1)),  \
+                                                                                                                bi_combine_pins(1u << (p2), 1u << (p3))), \
+                                                                                                name0 "|" name1 "|" name2 "|" name3)
 
 #endif

[will-v-pi]

Merged into develop #1858