[PATCH v4 05/25] VLAN: Create new data type for VLAN description.

Jouni Malinen j at w1.fi
Sun Aug 4 14:54:27 EDT 2013


On Sat, Jul 27, 2013 at 09:54:51PM +0200, Michael Braun wrote:
> This hides away the details of the currently in-use VLAN model
> and is preparing for adding tagged VLAN support later on.
> Implementing this as inline functions lets the compiler create
> as fast code as before the change.

>  src/common/vlan.h |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 src/common/vlan.h

Hmm.. This should likely be merged with some other commit(s) since on
its own, adding a new header file doesn't do anything.

> diff --git a/src/common/vlan.h b/src/common/vlan.h
> +#define VLAN_NULL 0
> +typedef int vlan_t;

Hmm.. I don't think I really like this. In general, there is desire to
avoid typedefs and specially typedefs for structures (which is what this
turns into further along in the series). 

Wouldn't it be simpler to just use struct vlan_description instead of
vlan_t and #ifdef CONFIG_VLAN_TAGGED deciding which contents the
structure has?

> +static inline void vlan_alloc(vlan_t *dst, const int untagged)
> +{
> +	*dst = untagged;
> +}

It does not look necessary to make these inline functions either. VLAN
management operations cannot really be that time critical that the
comment about compiler producing as fast code as before would make a
real difference. The versions added in 16/25 looks way too long to
justify inline functions in a header file.

-- 
Jouni Malinen                                            PGP id EFC895FA


More information about the HostAP mailing list