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

[Paolo Bonzini <pbonzini@redhat.com>]

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

> 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". :)

Paolo