Page MenuHomeFreeBSD

ng_hci: Cast NG_HCI_BDADDR_ANY to const bdaddr_t *
ClosedPublic

Authored by christos on Aug 31 2025, 6:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 10, 10:44 AM
Unknown Object (File)
Fri, Oct 10, 4:12 AM
Unknown Object (File)
Fri, Oct 10, 4:12 AM
Unknown Object (File)
Fri, Oct 10, 4:12 AM
Unknown Object (File)
Thu, Oct 9, 10:57 PM
Unknown Object (File)
Wed, Oct 8, 9:29 PM
Unknown Object (File)
Fri, Oct 3, 4:35 AM
Unknown Object (File)
Thu, Oct 2, 5:49 AM

Details

Summary

This is needed to address some compiler errors cleanly, where consumer
functions want this address to be a const pointer.

Sponsored by: The FreeBSD Foundation
MFC after: 3 days

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Not 100% sure if that's a correct fix.

sys/netgraph/bluetooth/include/ng_hci.h
452

What breaks if you make it a const bdaddr_p instead? We probably really don't want anything mutating it.

sys/netgraph/bluetooth/include/ng_hci.h
452

With this change it still compiles:

diff --git a/sys/netgraph/bluetooth/include/ng_hci.h b/sys/netgraph/bluetooth/include/ng_hci.h
index 19088d3ab13f..1ecc75c5affe 100644
--- a/sys/netgraph/bluetooth/include/ng_hci.h
+++ b/sys/netgraph/bluetooth/include/ng_hci.h
@@ -449,7 +449,7 @@ typedef bdaddr_t *  bdaddr_p;

 /* Any BD_ADDR. Note: This is actually 7 bytes (count '\0' terminator) */
 #define NG_HCI_BDADDR_ANY      \
-       ((bdaddr_p) __DECONST(char *, "\000\000\000\000\000\000"))
+       (__DECONST(const bdaddr_p, "\000\000\000\000\000\000"))
sys/netgraph/bluetooth/include/ng_hci.h
452

And what if you remove the __DECONST from that?

sys/netgraph/bluetooth/include/ng_hci.h
452

Without the __DECONST I get the same error as in the commit message.

sys/netgraph/bluetooth/include/ng_hci.h
452

Ok, so we want NG_HCI_BDADDR_ANY to be a pointer to const. Note that const bdaddr_p is a const pointer since bdaddr_p is a pointer type. So neither proposed patch is really right.

I'd suggest defining it this way instead: (&(const bdaddr_t){}). Maybe some of the consumers are not const-clean (e.g., sdp_register_service() looks wrong), those should be fixed (or __DECONST should be applied there) instead of mangling this definition.

sys/netgraph/bluetooth/include/ng_hci.h
452

Do you agree with defining:

/* Any BD_ADDR. Note: This is actually 7 bytes (count '\0' terminator) */
#define NG_HCI_BDADDR_ANY	(&(const bdaddr_t){"\000\000\000\000\000\000"})

as you said, which probably should be that way in the first place, and then simply __DECONSTing in the sdp_register_service() call in virtual_bt_speaker(8)?

	if (sdp_register_service(r->sdp_session, SDP_SERVICE_CLASS_AUDIO_SINK,
	    __DECONST(bdaddr_p, NG_HCI_BDADDR_ANY),
	    (const uint8_t *)&record, sizeof(record),
	    &r->sdp_handle)) {

There are a few more calls to this function in usr.bin/bluetooth/{rfcomm_sppd/rfcomm_sppd.c, btpand/server.c, rfcomm_pppd/rfcomm_pppd}.

sys/netgraph/bluetooth/include/ng_hci.h
452

I think that's ok. Or see if you can make sdp_register_service() take a pointer to const itself, and use __DECONST internally.

sys/netgraph/bluetooth/include/ng_hci.h
452

Why not have an btaddr_any global thats of the right type, initialized to 0? The the whole DECONST jank goes away, the weird cast goes away, etc. What is wrong with that more straight forward approach?

sys/netgraph/bluetooth/include/ng_hci.h
452

If that global is const, then I don't see how the DECONST jank goes away. If it's not const, then it's susceptible to accidental modification.

There is some precedent for your suggestion in in6addr_any and so on, but it requires that a symbol be defined somewhere. I don't see why it's better than the other proposals?

christos retitled this revision from ng_hci: Deconst NG_HCI_BDADDR_ANY to ng_hci: Cast NG_HCI_BDADDR_ANY to const bdaddr_t *.Sep 11 2025, 6:25 PM
christos edited the summary of this revision. (Show Details)
christos retitled this revision from ng_hci: Cast NG_HCI_BDADDR_ANY to const bdaddr_t * to ng_hci: Define NG_HCI_BDADDR_ANY as const bdaddr_t *.Sep 11 2025, 6:32 PM

@markj git-arc(1) for some reason doesn't let me update the diff (maybe the title is the issue?), so here is the new diff:

diff --git a/sys/netgraph/bluetooth/include/ng_hci.h b/sys/netgraph/bluetooth/include/ng_hci.h
index 44a14e62f4ed..ce3291770740 100644
--- a/sys/netgraph/bluetooth/include/ng_hci.h
+++ b/sys/netgraph/bluetooth/include/ng_hci.h
@@ -448,7 +448,7 @@ typedef struct {
 typedef bdaddr_t *     bdaddr_p;

 /* Any BD_ADDR. Note: This is actually 7 bytes (count '\0' terminator) */
-#define NG_HCI_BDADDR_ANY      ((bdaddr_p) "\000\000\000\000\000\000")
+#define NG_HCI_BDADDR_ANY      (&(const bdaddr_t){"\000\000\000\000\000\000"})

 /* HCI status return parameter */
 typedef struct {

@markj git-arc(1) for some reason doesn't let me update the diff (maybe the title is the issue?), so here is the new diff:

diff --git a/sys/netgraph/bluetooth/include/ng_hci.h b/sys/netgraph/bluetooth/include/ng_hci.h
index 44a14e62f4ed..ce3291770740 100644
--- a/sys/netgraph/bluetooth/include/ng_hci.h
+++ b/sys/netgraph/bluetooth/include/ng_hci.h
@@ -448,7 +448,7 @@ typedef struct {
 typedef bdaddr_t *     bdaddr_p;

 /* Any BD_ADDR. Note: This is actually 7 bytes (count '\0' terminator) */
-#define NG_HCI_BDADDR_ANY      ((bdaddr_p) "\000\000\000\000\000\000")
+#define NG_HCI_BDADDR_ANY      (&(const bdaddr_t){"\000\000\000\000\000\000"})

 /* HCI status return parameter */
 typedef struct {

Bump.

@markj git-arc(1) for some reason doesn't let me update the diff (maybe the title is the issue?), so here is the new diff:

The commit and review titles have to match.

diff --git a/sys/netgraph/bluetooth/include/ng_hci.h b/sys/netgraph/bluetooth/include/ng_hci.h
index 44a14e62f4ed..ce3291770740 100644
--- a/sys/netgraph/bluetooth/include/ng_hci.h
+++ b/sys/netgraph/bluetooth/include/ng_hci.h
@@ -448,7 +448,7 @@ typedef struct {
 typedef bdaddr_t *     bdaddr_p;

 /* Any BD_ADDR. Note: This is actually 7 bytes (count '\0' terminator) */
-#define NG_HCI_BDADDR_ANY      ((bdaddr_p) "\000\000\000\000\000\000")
+#define NG_HCI_BDADDR_ANY      (&(const bdaddr_t){"\000\000\000\000\000\000"})

 /* HCI status return parameter */
 typedef struct {

Ok.

This revision is now accepted and ready to land.Sep 15 2025, 1:05 PM
christos retitled this revision from ng_hci: Define NG_HCI_BDADDR_ANY as const bdaddr_t * to ng_hci: Cast NG_HCI_BDADDR_ANY to const bdaddr_t *.Sep 15 2025, 1:08 PM