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

[Jan Kiszka]

On 2012-03-22 01:02, Paolo Bonzini wrote:
> Right now, slirp/slirp.h cannot include some system headers and,
> indirectly, qemu_socket.h.  Clean this up, and remove a duplicate
> prototype that was introduced because of that.
> 
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  slirp/slirp.h |    8 +-------
>  slirp/tcp.h   |   21 +++++++++++++++------
>  2 files changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/slirp/slirp.h b/slirp/slirp.h
> index 5033ee3..2098b20 100644
> --- a/slirp/slirp.h
> +++ b/slirp/slirp.h
> @@ -88,10 +88,6 @@ void *malloc(size_t arg);
>  void free(void *ptr);
>  #endif
>  
> -#ifndef HAVE_INET_ATON
> -int inet_aton(const char *cp, struct in_addr *ia);
> -#endif
> -
>  #include <fcntl.h>
>  #ifndef NO_UNIX_SOCKETS
>  #include <sys/un.h>
> @@ -144,6 +140,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
>  #include "debug.h"
>  
>  #include "qemu-queue.h"
> +#include "qemu_socket.h"
>  
>  #include "libslirp.h"
>  #include "ip.h"
> @@ -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? Even
better would be enabling slirp to use an existing declaration. But that
looks trickier in first sight.

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

>  
>  /*
>   * 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. ;)

Patch 2 looks ok, btw.

Thanks,
Jan

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