Re: [Qemu-devel] [PATCH 1/2] slirp: clean up conflicts with system headers

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-03-22 20:24, Paolo Bonzini wrote:
> Il 22/03/2012 15:35, Jan Kiszka ha scritto:
>>> @@ -167,9 +164,6 @@ int inet_aton(const char *cp, struct in_addr *ia);
>>>  #include "bootp.h"
>>>  #include "tftp.h"
>>>  
>>> -/* osdep.c */
>>> -int qemu_socket(int domain, int type, int protocol);
>>> -
>>>  #define ETH_ALEN 6
>>>  #define ETH_HLEN 14
>>>  
>>> diff --git a/slirp/tcp.h b/slirp/tcp.h
>>> index b3817cb..8299603 100644
>>> --- a/slirp/tcp.h
>>> +++ b/slirp/tcp.h
>>> @@ -45,6 +45,7 @@ typedef	uint32_t tcp_seq;
>>>   * TCP header.
>>>   * Per RFC 793, September, 1981.
>>>   */
>>> +#define tcphdr slirp_tcphdr
>>
>> Nice :). What about s/tcphdr/bsd_tcphdr/ or so for all slirp files?
> 
> Well, we have a precedent here:
> 
> /* Avoid conflicting with the libc insque() and remque(), which
>    have different prototypes. */
> #define insque slirp_insque
> #define remque slirp_remque

I know...

> 
>> Even better would be enabling slirp to use an existing declaration. But that
>> looks trickier in first sight.
> 
> Yep, especially with no Autoconf.
> 
>>>  struct tcphdr {
>>>  	uint16_t th_sport;              /* source port */
>>>  	uint16_t th_dport;              /* destination port */
>>> @@ -58,12 +59,6 @@ struct tcphdr {
>>>  		th_off:4;		/* data offset */
>>>  #endif
>>>  	uint8_t th_flags;
>>> -#define	TH_FIN	0x01
>>> -#define	TH_SYN	0x02
>>> -#define	TH_RST	0x04
>>> -#define	TH_PUSH	0x08
>>> -#define	TH_ACK	0x10
>>> -#define	TH_URG	0x20
>>>  	uint16_t th_win;                /* window */
>>>  	uint16_t th_sum;                /* checksum */
>>>  	uint16_t th_urp;                /* urgent pointer */
>>> @@ -71,6 +66,16 @@ struct tcphdr {
>>>  
>>>  #include "tcp_var.h"
>>>  
>>> +#ifndef TH_FIN
>>> +#define	TH_FIN	0x01
>>> +#define	TH_SYN	0x02
>>> +#define	TH_RST	0x04
>>> +#define	TH_PUSH	0x08
>>> +#define	TH_ACK	0x10
>>> +#define	TH_URG	0x20
>>> +#endif
>>> +
>>> +#ifndef TCPOPT_EOL
>>>  #define	TCPOPT_EOL		0
>>>  #define	TCPOPT_NOP		1
>>>  #define	TCPOPT_MAXSEG		2
>>> @@ -86,6 +91,7 @@ struct tcphdr {
>>>  
>>>  #define TCPOPT_TSTAMP_HDR	\
>>>      (TCPOPT_NOP<<24|TCPOPT_NOP<<16|TCPOPT_TIMESTAMP<<8|TCPOLEN_TIMESTAMP)
>>> +#endif
>>
>> Is there no portable header that offers those defines for us?
> 
> That would be netinet/tcp.h, but the problem is that there are some
> differences.  For example TCP_MSS is usually 512.
> 
> BTW the same could happen for ip.h, it's only that we never include it.
> We need to include netinet/tcp.h outside slirp for TCP_NODELAY.
> 
>>>  /*
>>>   * Default maximum segment size for TCP.
>>> @@ -95,10 +101,13 @@ struct tcphdr {
>>>   *
>>>   * We make this 1460 because we only care about Ethernet in the qemu context.
>>>   */
>>> +#undef TCP_MSS
>>>  #define	TCP_MSS	1460
>>>  
>>> +#undef TCP_MAXWIN
>>>  #define	TCP_MAXWIN	65535	/* largest value for (unscaled) window */
>>>  
>>> +#undef TCP_MAX_WINSHIFT
>>>  #define TCP_MAX_WINSHIFT	14	/* maximum window shift */
>>>  
>>>  /*
>>
>> Same here.
>>
>> The direction is appreciated a lot, but I'm measuring only a moderate
>> overall hack-level reduction. ;)
> 
> Agreed, but one step at a time... this patch sticks to what it
> promises, "clean up conflicts with system headers". :)

Yeah, given that there is no sufficiently simple alternative, I guess I
have to accept this as is. :)

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux