Re: [Qemu-devel] [PATCH 1/2] slirp: remove duplicate definition

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

On 2012-02-15 10:38, Michael S. Tsirkin wrote:
> On Wed, Feb 15, 2012 at 09:30:23AM +0100, Jan Kiszka wrote:
>> On 2012-02-15 09:12, zwu.kernel@gmail.com wrote:
>>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>>
>>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>> ---
>>>  slirp/if.c |    2 --
>>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/slirp/if.c b/slirp/if.c
>>> index 2852396..8e0cac2 100644
>>> --- a/slirp/if.c
>>> +++ b/slirp/if.c
>>> @@ -8,8 +8,6 @@
>>>  #include <slirp.h>
>>>  #include "qemu-timer.h"
>>>  
>>> -#define ifs_init(ifm) ((ifm)->ifs_next = (ifm)->ifs_prev = (ifm))
>>> -
>>>  static void
>>>  ifs_insque(struct mbuf *ifm, struct mbuf *ifmhead)
>>>  {
>>
>> Let's grab the chance and move ifs_init to mbuf.h.
>>
>> Jan
>>
> 
> Since you mention it - why does slirp have all these
> defines in the first place?
> 
> slirp/mbuf.h:#define m_nextpkt  m_hdr.mh_nextpkt
> slirp/mbuf.h:#define ifs_next m_nextpkt
> 
> Seriously, #define for a field name?
> This is just crazy, and violates our coding style
> which requires macros to be PPER_CAS_WITH_UNDERSCORES
> 

This is a total mess, I agree, and I'd warmly welcome patches to clean
this up.

I can only speculate on the reasons. Maybe the intention was to keep the
core minimally modified and wrap all the conflicts it causes away. Given
that slirp outside QEMU is dead, this is no longer a valid reason (if it
ever was).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]