@hci.h use of magic-number in bluetooth addr struct (Missing define in @bluetooth.h)

[TTJO]

In bluetooth/hci.h the bluetooth address type is declared as

typedef struct {
	u8_t  val[6];
} bt_addr_t;

Instead of using magic number '6' this should be declared as a define in bluetooth.h:
#define BT_ADDR_LEN (6)

[jhedberg]

I'm not convinced it's worth it here. A Bluetooth address is defined to be six octets long, and that's what the struct definition demonstrates. There really isn't any "magic" about the number the way it's used here. Also, a define like that would not be used anywhere else (instead sizeof(bt_addr_t) should be used).

[TTJO]

I think we can discuss the definition and use of Magic Numbers for a long time :)
However this might not be used anywhere else in HCI or Link Layer, but it would be a nice define to have available in the Application layer for different purposes (eg. copy or compare) of iterating over the BT address.
It makes for more readable code than using sizeof multiple places.

[jhedberg]

@TTJO there are already bt_addr_copy and bt_addr_cmp APIs, if you were looking for something like that. Same for the bt_addr_le type.

[jhedberg]

@TTJO I'd also like to add that one of the reasons these are typedefs instead of using them just as any other structs is specifically to highlight the fact that the internals is something the user should never (or extremely rarely) have to touch or care about.

[jhedberg]

@TTJO any feedback on my latest comments? I'd like to know whether I should close this issue or if there's still something unresolved. Thanks.

[jhedberg]

(added some maintainers as reviewers in case they want to voice their thoughts on this)

[Vudentz]

I would have to agree with @jhedberg if we do have a struct representing a bluetooth address the application should use sizeof that struct instead of a define, anyway sizeof will be evaluated at build time so in practice there is no real difference to a define except that sizeof would actually be more precise if the address ever change e.g. due to a bug.

[cvinayak]

I too agree with @jhedberg, we keep sizeof(bt_addr_t).