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

[michael-dev]

Am 04.08.2013 20:54, schrieb Jouni Malinen:
> On Sat, Jul 27, 2013 at 09:54:51PM +0200, Michael Braun wrote:
>>  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.

The problem was that the commit to add this header and actually use the 
new data type in one was too big (to be sent on this mailing list).
So I splitted it up and one of the changes left alone was adding this 
header file.
I don't really know which commit should be merged with this?

> 
>> 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?

The beauty of having a kind of alias between vlan_t and int is that for 
the commits that migrate from int to vlan_t, the code after each single 
commit will still compile and run fine. This is usefull if somebody for 
example wants to bisect.
A typedef is not really needed, a #define vlan_t int would work just as 
well, though that doesn't really make things better.
So maybe have some commits after which the code doesn't compile just 
cannot be avoided.

> 
>> +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.

I'll change that.

Regards,
  M. Braun