| Message ID | 20161105000640.49839-1-pallas@meraki.com |
|---|---|
| State | Rejected |
| Headers | show |
Hi, vlan_id.tagged is int[MAX_NUM_TAGGED_VLAN]. I tested the assignments with a minimal test case (attached) using struct vlan_description with gcc 4.9.2. 1. int * tagged = vlan_id.tagged -> accepted without warning 2. int * tagged = &vlan_id.tagged -> warning: initialization from incompatible pointer type So I think the patch not correct. But interestingly, both vlan_id.tagged and &vlan_id.taged resolve to the same pointer value for me. Regards, M. Braun Am 05.11.2016 um 01:06 schrieb Derrick Pallas: > Discovered & fixed by Louisa Chong, `tagged` is an `int*`, not an `int`, so > we actually want to take the address of `cache->vlan_id.tagged` here. > > Signed-off-by: Louisa Chong <clouisa@meraki.com> > Signed-off-by: Derrick Pallas <pallas@meraki.com> > --- > src/ap/ieee802_11_auth.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ap/ieee802_11_auth.c b/src/ap/ieee802_11_auth.c > index b890537..86230c9 100644 > --- a/src/ap/ieee802_11_auth.c > +++ b/src/ap/ieee802_11_auth.c > @@ -569,7 +569,7 @@ hostapd_acl_recv_radius(struct radius_msg *msg, struct radius_msg *req, > > notempty = &cache->vlan_id.notempty; > untagged = &cache->vlan_id.untagged; > - tagged = cache->vlan_id.tagged; > + tagged = &cache->vlan_id.tagged; > *notempty = !!radius_msg_get_vlanid(msg, untagged, > MAX_NUM_TAGGED_VLAN, > tagged); >
On Fri, Nov 04, 2016 at 05:06:40PM -0700, Derrick Pallas wrote: > Discovered & fixed by Louisa Chong, `tagged` is an `int*`, not an `int`, so > we actually want to take the address of `cache->vlan_id.tagged` here. cache->vlan_id.tagged is an array (int tagged[32]), the current code gets its address; not the cache->vlan_id.tagged[0] value or any other int for that matter. > diff --git a/src/ap/ieee802_11_auth.c b/src/ap/ieee802_11_auth.c > @@ -569,7 +569,7 @@ hostapd_acl_recv_radius(struct radius_msg *msg, struct radius_msg *req, > - tagged = cache->vlan_id.tagged; > + tagged = &cache->vlan_id.tagged; This would break the build with -Werror: ../src/ap/ieee802_11_auth.c: In function ‘hostapd_acl_recv_radius’: ../src/ap/ieee802_11_auth.c:572:10: error: assignment from incompatible pointer type [-Werror] tagged = &cache->vlan_id.tagged; The current cache->vlan_id.tagged here is already getting the correct pointer to the first entry in the array, i.e., it has the same value as &cache->vlan_id.tagged[0]. While getting the address of the array with & would have the same address value, it would have different type and that's not what we want here.
diff --git a/src/ap/ieee802_11_auth.c b/src/ap/ieee802_11_auth.c index b890537..86230c9 100644 --- a/src/ap/ieee802_11_auth.c +++ b/src/ap/ieee802_11_auth.c @@ -569,7 +569,7 @@ hostapd_acl_recv_radius(struct radius_msg *msg, struct radius_msg *req, notempty = &cache->vlan_id.notempty; untagged = &cache->vlan_id.untagged; - tagged = cache->vlan_id.tagged; + tagged = &cache->vlan_id.tagged; *notempty = !!radius_msg_get_vlanid(msg, untagged, MAX_NUM_TAGGED_VLAN, tagged);