diff mbox

[for-1.1?] slirp: Avoid redefining MAX_TCPOPTLEN

Message ID 1338138128-53411-1-git-send-email-andreas.faerber@web.de
State New
Headers show

Commit Message

Andreas Färber May 27, 2012, 5:02 p.m. UTC
MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40, causing a
warning. The value is only used to declare an array, into which currently
4 bytes are written at most. It should therefore be acceptable to adopt
the host's definition.

Therefore only define MAX_TCPOPTLEN if not already defined.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 slirp/tcp_output.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Paolo Bonzini May 28, 2012, 6:49 a.m. UTC | #1
Il 27/05/2012 19:02, Andreas Färber ha scritto:
> MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40, causing a
> warning. The value is only used to declare an array, into which currently
> 4 bytes are written at most. It should therefore be acceptable to adopt
> the host's definition.
> 
> Therefore only define MAX_TCPOPTLEN if not already defined.
> 
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  slirp/tcp_output.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
> index 779314b..9815123 100644
> --- a/slirp/tcp_output.c
> +++ b/slirp/tcp_output.c
> @@ -47,7 +47,9 @@ static const u_char  tcp_outflags[TCP_NSTATES] = {
>  };
>  
>  
> +#ifndef MAX_TCPOPTLEN
>  #define MAX_TCPOPTLEN	32	/* max # bytes that go in options */
> +#endif
>  
>  /*
>   * Tcp output routine: figure out what should be sent and send it.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo
Stefan Weil May 28, 2012, 7 a.m. UTC | #2
Am 28.05.2012 08:49, schrieb Paolo Bonzini:
> Il 27/05/2012 19:02, Andreas Färber ha scritto:
>> MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40, causing a
>> warning. The value is only used to declare an array, into which currently
>> 4 bytes are written at most. It should therefore be acceptable to adopt
>> the host's definition.

Can we be sure that there will be never a build environment
which defines MAX_TCPOPTLENto a value less than 4?

If not, either the preprocessor or an assertion should test
that, or we could use QEMU_MAX_TCPOPTLEN or SLIRP_MAX_TCPOPTLEN.

Regards,

Stefan W.
Jan Kiszka May 28, 2012, 11:40 a.m. UTC | #3
On 2012-05-27 14:02, Andreas Färber wrote:
> MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40, causing a
> warning. The value is only used to declare an array, into which currently
> 4 bytes are written at most. It should therefore be acceptable to adopt
> the host's definition.
> 
> Therefore only define MAX_TCPOPTLEN if not already defined.
> 
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>  slirp/tcp_output.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
> index 779314b..9815123 100644
> --- a/slirp/tcp_output.c
> +++ b/slirp/tcp_output.c
> @@ -47,7 +47,9 @@ static const u_char  tcp_outflags[TCP_NSTATES] = {
>  };
>  
>  
> +#ifndef MAX_TCPOPTLEN
>  #define MAX_TCPOPTLEN	32	/* max # bytes that go in options */
> +#endif
>  
>  /*
>   * Tcp output routine: figure out what should be sent and send it.

Let's be conservative for 1.1 and do #undef MAX_TCPOPTLEN instead. Does
this work as well?

Jan
Andreas Färber May 28, 2012, 2:45 p.m. UTC | #4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 28.05.2012 13:40, schrieb Jan Kiszka:
> On 2012-05-27 14:02, Andreas Färber wrote:
>> MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40,
>> causing a warning. The value is only used to declare an array,
>> into which currently 4 bytes are written at most. It should
>> therefore be acceptable to adopt the host's definition.
>> 
>> Therefore only define MAX_TCPOPTLEN if not already defined.
>> 
>> Signed-off-by: Andreas Färber <andreas.faerber@web.de> --- 
>> slirp/tcp_output.c |    2 ++ 1 files changed, 2 insertions(+), 0
>> deletions(-)
>> 
>> diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c index
>> 779314b..9815123 100644 --- a/slirp/tcp_output.c +++
>> b/slirp/tcp_output.c @@ -47,7 +47,9 @@ static const u_char
>> tcp_outflags[TCP_NSTATES] = { };
>> 
>> 
>> +#ifndef MAX_TCPOPTLEN #define MAX_TCPOPTLEN	32	/* max # bytes
>> that go in options */ +#endif
>> 
>> /* * Tcp output routine: figure out what should be sent and send
>> it.
> 
> Let's be conservative for 1.1 and do #undef MAX_TCPOPTLEN instead.
> Does this work as well?

For Leopard I'd guess so.

What I had in mind here was that it was suggested earlier by one of
you to use host defines for 1.2 wherever possible, and this seemed
like low-hanging fruit. I do see Stefan's point though.

An alternative, host-overriding version would be:

#if !defined(MAX_TCPOPTLEN) || MAX_TCPOPTLEN < 4
#undef MAX_TCPOPTLEN
...
#endif

Stefan's point in turn means that defining MAX_TCPOPTLEN to something
larger than the host supports could in theory cause trouble when used
with host functions (which we currently don't seem to).

A safe host-adopting version would be:

#if defined(MAX_TCPOPTLEN) && MAX_TCPOPTLEN >= 4
#define SLIRP_MAX_TCPOPTLEN MAX_TCPOPTLEN
#else
#define SLIRP_MAX_TCPOPTLEN 32
#endif

and use opt[SLIRP_MAX_TCPOPTLEN]. In that case we could probably just
rename MAX_TCPOPTLEN -> SLIRP_MAX_TCPOPTLEN in the first place.

Yet another option would be to drop MAX_TCPOPTLEN and to hardcode the
actually used opt[4].

Which do you prefer? :-)

Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)

iQIcBAEBAgAGBQJPw49tAAoJEPou0S0+fgE/o5cP+gKOS0NJVOWl+XOIB1Pb+xgs
WZ7f5Uiv0zAuzzcb4VLsB4XcWrxdkKdE2X284OFQyZIeya6IWuHuh35XCDDo1IG7
NwoV/rlZVRG3ttFEG1+6H49dgx/UeveyCs8kKsUQahO2teq423M4fStWhxky8SsQ
sxPoJZTw5gOAyQLzGPO8kZ+3jXwxm9wgsEYvjhqKz7HVHuxcy2EHbYDxYeuRtJnf
orwaUEVrGfwOwjDZF4E3R+x8z6YFjQ6a2/zsNnzeSzyXe67Mtmwfq/HSBmYpl6pY
YQSAxLrrOUL5jCXzckL8GoW4YKfNgsz8rcqK5RRBI27JPVQj+bdVJSbR0N6E4EAT
50u6uFSWV/uJFMu0f83fC2uiEt8c0i3dYDNmWZXKytdl9KfHu2SQiVqV7GfxcszT
Q4x0S9qnjB1P+Sh8Gtz61r8OQEAd9HssEaEbruzddvjXwuMJizcQKUZunO7PPx8V
EkWKIDwg/hAcIVody1j3T6llShc+LVfwDmaajFhLPK9lqm6sw15HwZ9V9E7ww0GD
DxRvVYud/XL4Zn6XpJVfozgQyAwWYKDduEZNIxA6YlIgtignXA8X+0wpRGKAJhea
ttIO9PBpeOMTGhw5rpvo1MtyPrQXyk0XKNpL/vpseQfnt3x9Asfww0qYrTLlejuW
X1ukGz9cleIqZyog5IHQ
=qYtt
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
index 779314b..9815123 100644
--- a/slirp/tcp_output.c
+++ b/slirp/tcp_output.c
@@ -47,7 +47,9 @@  static const u_char  tcp_outflags[TCP_NSTATES] = {
 };
 
 
+#ifndef MAX_TCPOPTLEN
 #define MAX_TCPOPTLEN	32	/* max # bytes that go in options */
+#endif
 
 /*
  * Tcp output routine: figure out what should be sent and send it.