src: remove dependency on libiptc headers

src: remove dependency on libiptc headers

[Jan Engelhardt]

commit 8219a9d65fe583de06c2ff4e84d1ea299955184b
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Wed Jul 30 08:36:33 2008 -0400

src: remove dependency on libiptc headers

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_TCPOPTSTRIP.c         |    1 +
 extensions/libxt_hashlimit.c           |    1 +
 extensions/libxt_time.c                |    1 +
 extensions/tos_values.c                |    1 +
 include/ip6tables.h                    |    4 ++--
 include/iptables.h                     |    5 +++--
 include/libiptc/libxtc.h               |    2 --
 include/linux/netfilter/xt_RATEEST.h   |    2 ++
 include/linux/netfilter/xt_hashlimit.h |    4 ++--
 include/linux/netfilter/xt_physdev.h   |    8 ++++----
 include/linux/netfilter/xt_rateest.h   |    4 ++--
 include/xtables.h.in                   |    8 +++++---
 xtables.c                              |    1 +
 13 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/extensions/libxt_TCPOPTSTRIP.c b/extensions/libxt_TCPOPTSTRIP.c
index bd74e37..a00c82b 100644
--- a/extensions/libxt_TCPOPTSTRIP.c
+++ b/extensions/libxt_TCPOPTSTRIP.c
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <xtables.h>
+#include <netinet/tcp.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_TCPOPTSTRIP.h>
 #ifndef TCPOPT_MD5SIG
diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c
index 2f6b3fc..1f34fb9 100644
--- a/extensions/libxt_hashlimit.c
+++ b/extensions/libxt_hashlimit.c
@@ -11,6 +11,7 @@
  * Error corections by nmalykh@bilim.com (22.01.2005)
  */
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
diff --git a/extensions/libxt_time.c b/extensions/libxt_time.c
index 97bb0d3..f2bb51f 100644
--- a/extensions/libxt_time.c
+++ b/extensions/libxt_time.c
@@ -12,6 +12,7 @@
 #include <sys/types.h>
 #include <getopt.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
diff --git a/extensions/tos_values.c b/extensions/tos_values.c
index 4c0b0bc..0ab784d 100644
--- a/extensions/tos_values.c
+++ b/extensions/tos_values.c
@@ -1,6 +1,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <linux/ip.h>
 
 struct tos_value_mask {
 	uint8_t value, mask;
diff --git a/include/ip6tables.h b/include/ip6tables.h
index 077fee9..dfbc9b2 100644
--- a/include/ip6tables.h
+++ b/include/ip6tables.h
@@ -1,9 +1,9 @@
 #ifndef _IP6TABLES_USER_H
 #define _IP6TABLES_USER_H
 
+#include <netinet/ip.h>
 #include <xtables.h>
-
-#include "libiptc/libip6tc.h"
+#include <libiptc/libip6tc.h>
 
 #ifndef IP6T_SO_GET_REVISION_MATCH /* Old kernel source. */
 #define IP6T_SO_GET_REVISION_MATCH	68
diff --git a/include/iptables.h b/include/iptables.h
index ecc7168..99e8e1e 100644
--- a/include/iptables.h
+++ b/include/iptables.h
@@ -1,8 +1,9 @@
 #ifndef _IPTABLES_USER_H
 #define _IPTABLES_USER_H
 
-#include "xtables.h"
-#include "libiptc/libiptc.h"
+#include <netinet/ip.h>
+#include <xtables.h>
+#include <libiptc/libiptc.h>
 
 #ifndef IPT_SO_GET_REVISION_MATCH /* Old kernel source. */
 #define IPT_SO_GET_REVISION_MATCH	(IPT_BASE_CTL + 2)
diff --git a/include/libiptc/libxtc.h b/include/libiptc/libxtc.h
index 031afb5..3701018 100644
--- a/include/libiptc/libxtc.h
+++ b/include/libiptc/libxtc.h
@@ -20,8 +20,6 @@ extern "C" {
 #define XT_ALIGN(s) (((s) + ((XT_MIN_ALIGN)-1)) & ~((XT_MIN_ALIGN)-1))
 #endif
 
-typedef char xt_chainlabel[32];
-
 #define XTC_LABEL_ACCEPT  "ACCEPT"
 #define XTC_LABEL_DROP    "DROP"
 #define XTC_LABEL_QUEUE   "QUEUE"
diff --git a/include/linux/netfilter/xt_RATEEST.h b/include/linux/netfilter/xt_RATEEST.h
index f79e313..59b0257 100644
--- a/include/linux/netfilter/xt_RATEEST.h
+++ b/include/linux/netfilter/xt_RATEEST.h
@@ -1,6 +1,8 @@
 #ifndef _XT_RATEEST_TARGET_H
 #define _XT_RATEEST_TARGET_H
 
+#include <linux/if.h>
+
 struct xt_rateest_target_info {
 	char			name[IFNAMSIZ];
 	int8_t			interval;
diff --git a/include/linux/netfilter/xt_hashlimit.h b/include/linux/netfilter/xt_hashlimit.h
index 51b18d8..7e44c57 100644
--- a/include/linux/netfilter/xt_hashlimit.h
+++ b/include/linux/netfilter/xt_hashlimit.h
@@ -30,7 +30,7 @@ struct hashlimit_cfg {
 };
 
 struct xt_hashlimit_info {
-	char name [IFNAMSIZ];		/* name */
+	char name[16];		/* name */
 	struct hashlimit_cfg cfg;
 
 	/* Used internally by the kernel */
@@ -56,7 +56,7 @@ struct hashlimit_cfg1 {
 };
 
 struct xt_hashlimit_mtinfo1 {
-	char name[IFNAMSIZ];
+	char name[16];
 	struct hashlimit_cfg1 cfg;
 
 	/* Used internally by the kernel */
diff --git a/include/linux/netfilter/xt_physdev.h b/include/linux/netfilter/xt_physdev.h
index 9d33619..3500a50 100644
--- a/include/linux/netfilter/xt_physdev.h
+++ b/include/linux/netfilter/xt_physdev.h
@@ -10,10 +10,10 @@
 #define XT_PHYSDEV_OP_MASK		(0x20 - 1)
 
 struct xt_physdev_info {
-	char physindev[IFNAMSIZ];
-	char in_mask[IFNAMSIZ];
-	char physoutdev[IFNAMSIZ];
-	char out_mask[IFNAMSIZ];
+	char physindev[16];
+	char in_mask[16];
+	char physoutdev[16];
+	char out_mask[16];
 	u_int8_t invert;
 	u_int8_t bitmask;
 };
diff --git a/include/linux/netfilter/xt_rateest.h b/include/linux/netfilter/xt_rateest.h
index 2010cb7..4f7c071 100644
--- a/include/linux/netfilter/xt_rateest.h
+++ b/include/linux/netfilter/xt_rateest.h
@@ -18,8 +18,8 @@ enum xt_rateest_match_mode {
 };
 
 struct xt_rateest_match_info {
-	char			name1[IFNAMSIZ];
-	char			name2[IFNAMSIZ];
+	char			name1[16];
+	char			name2[16];
 	u_int16_t		flags;
 	u_int16_t		mode;
 	u_int32_t		bps1;
diff --git a/include/xtables.h.in b/include/xtables.h.in
index 4ce73e9..9dc91d4 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -1,10 +1,10 @@
 #ifndef _XTABLES_H
 #define _XTABLES_H
 
+#include <sys/socket.h> /* PF_* */
 #include <sys/types.h>
 #include <linux/types.h>
 #include <linux/netfilter/x_tables.h>
-#include <libiptc/libxtc.h>
 #include <stdbool.h>
 
 #ifndef IPPROTO_SCTP
@@ -22,12 +22,14 @@
 
 #define XTABLES_API_VERSION(x,y,z)    (0x10000*(x) + 0x100*(y) + z)
 
+struct in_addr;
+
 /* Include file for additions: new matches and targets. */
 struct xtables_match
 {
 	struct xtables_match *next;
 
-	xt_chainlabel name;
+	char name[32];
 
 	/* Revision of match (0 by default). */
 	u_int8_t revision;
@@ -83,7 +85,7 @@ struct xtables_target
 {
 	struct xtables_target *next;
 
-	xt_chainlabel name;
+	char name[32];
 
 	/* Revision of target (0 by default). */
 	u_int8_t revision;
diff --git a/xtables.c b/xtables.c
index a97bdaa..abdd283 100644
--- a/xtables.c
+++ b/xtables.c
@@ -32,6 +32,7 @@
 #include <arpa/inet.h>
 
 #include <xtables.h>
+#include <libiptc/libxtc.h>
 
 #ifndef NO_SHARED_LIBS
 #include <dlfcn.h>

Re: src: remove dependency on libiptc headers

[Patrick McHardy]

Jan Engelhardt wrote:
> commit 8219a9d65fe583de06c2ff4e84d1ea299955184b
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date:   Wed Jul 30 08:36:33 2008 -0400
> 
> src: remove dependency on libiptc headers
> 
>  struct xt_rateest_match_info {
> -	char			name1[IFNAMSIZ];
> -	char			name2[IFNAMSIZ];
> +	char			name1[16];
> +	char			name2[16];

Seems unrelated and I don't want to apply this change.
Please remove the IFNAMSIZ changes.

>  struct xtables_match
>  {
>  	struct xtables_match *next;
>  
> -	xt_chainlabel name;
> +	char name[32];
>  

Why 32?

Re: src: remove dependency on libiptc headers

[Jan Engelhardt]

On Wednesday 2008-07-30 09:45, Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> commit 8219a9d65fe583de06c2ff4e84d1ea299955184b
>> Author: Jan Engelhardt <jengelh@medozas.de>
>> Date:   Wed Jul 30 08:36:33 2008 -0400
>> 
>> src: remove dependency on libiptc headers
>> 
>> struct xt_rateest_match_info {
>> -	char			name1[IFNAMSIZ];
>> -	char			name2[IFNAMSIZ];
>> +	char			name1[16];
>> +	char			name2[16];
>
> Seems unrelated and I don't want to apply this change.
> Please remove the IFNAMSIZ changes.

Then I would have to add linux/if.h everywhere.

>>  struct xtables_match
>>  {
>>   struct xtables_match *next;
>> 
>> -	xt_chainlabel name;
>> +	char name[32];
>>  
>
> Why 32?
>
That is what xt_chainlabel was typedefed to. No clue who had that
idea, char *name would work just as well.

Re: src: remove dependency on libiptc headers

[Patrick McHardy]

Jan Engelhardt wrote:
> On Wednesday 2008-07-30 09:45, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> commit 8219a9d65fe583de06c2ff4e84d1ea299955184b
>>> Author: Jan Engelhardt <jengelh@medozas.de>
>>> Date:   Wed Jul 30 08:36:33 2008 -0400
>>>
>>> src: remove dependency on libiptc headers
>>>
>>> struct xt_rateest_match_info {
>>> -	char			name1[IFNAMSIZ];
>>> -	char			name2[IFNAMSIZ];
>>> +	char			name1[16];
>>> +	char			name2[16];
>> Seems unrelated and I don't want to apply this change.
>> Please remove the IFNAMSIZ changes.
> 
> Then I would have to add linux/if.h everywhere.

Fine with me. Going towards more magic values is never
a good idea :)

Re: src: remove dependency on libiptc headers

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Jan Engelhardt wrote:
>> On Wednesday 2008-07-30 09:45, Patrick McHardy wrote:
>>> Jan Engelhardt wrote:
>>>> commit 8219a9d65fe583de06c2ff4e84d1ea299955184b
>>>> Author: Jan Engelhardt <jengelh@medozas.de>
>>>> Date:   Wed Jul 30 08:36:33 2008 -0400
>>>>
>>>> src: remove dependency on libiptc headers
>>>>
>>>> struct xt_rateest_match_info {
>>>> -    char            name1[IFNAMSIZ];
>>>> -    char            name2[IFNAMSIZ];
>>>> +    char            name1[16];
>>>> +    char            name2[16];
>>> Seems unrelated and I don't want to apply this change.
>>> Please remove the IFNAMSIZ changes.
>>
>> Then I would have to add linux/if.h everywhere.
> 
> Fine with me. Going towards more magic values is never
> a good idea :)

Better include net/if.h which is include in a normal development setup
(libc6-dev package in debian).

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

src: remove dependency on libiptc headers

[Jan Engelhardt]

(patch 1/2)
commit 88496da7062bb0a2c07685a5b1968fc2ecf6d887
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Fri Aug 1 09:02:26 2008 -0400

src: remove dependency on libiptc headers

xtables.h does not need really need libxtc.h, and we can drop it from 
the install as it is internal-only.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_TCPOPTSTRIP.c |    1 +
 extensions/libxt_hashlimit.c   |    1 +
 extensions/libxt_time.c        |    1 +
 extensions/tos_values.c        |    1 +
 include/ip6tables.h            |    4 ++--
 include/iptables.h             |    5 +++--
 include/libiptc/libxtc.h       |    2 --
 include/xtables.h.in           |   11 +++++++----
 xtables.c                      |    1 +
 9 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/extensions/libxt_TCPOPTSTRIP.c b/extensions/libxt_TCPOPTSTRIP.c
index bd74e37..a00c82b 100644
--- a/extensions/libxt_TCPOPTSTRIP.c
+++ b/extensions/libxt_TCPOPTSTRIP.c
@@ -10,6 +10,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <xtables.h>
+#include <netinet/tcp.h>
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_TCPOPTSTRIP.h>
 #ifndef TCPOPT_MD5SIG
diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c
index 2f6b3fc..1f34fb9 100644
--- a/extensions/libxt_hashlimit.c
+++ b/extensions/libxt_hashlimit.c
@@ -11,6 +11,7 @@
  * Error corections by nmalykh@bilim.com (22.01.2005)
  */
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
diff --git a/extensions/libxt_time.c b/extensions/libxt_time.c
index 97bb0d3..f2bb51f 100644
--- a/extensions/libxt_time.c
+++ b/extensions/libxt_time.c
@@ -12,6 +12,7 @@
 #include <sys/types.h>
 #include <getopt.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
diff --git a/extensions/tos_values.c b/extensions/tos_values.c
index 4c0b0bc..0ab784d 100644
--- a/extensions/tos_values.c
+++ b/extensions/tos_values.c
@@ -1,6 +1,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <linux/ip.h>
 
 struct tos_value_mask {
 	uint8_t value, mask;
diff --git a/include/ip6tables.h b/include/ip6tables.h
index 077fee9..dfbc9b2 100644
--- a/include/ip6tables.h
+++ b/include/ip6tables.h
@@ -1,9 +1,9 @@
 #ifndef _IP6TABLES_USER_H
 #define _IP6TABLES_USER_H
 
+#include <netinet/ip.h>
 #include <xtables.h>
-
-#include "libiptc/libip6tc.h"
+#include <libiptc/libip6tc.h>
 
 #ifndef IP6T_SO_GET_REVISION_MATCH /* Old kernel source. */
 #define IP6T_SO_GET_REVISION_MATCH	68
diff --git a/include/iptables.h b/include/iptables.h
index ecc7168..99e8e1e 100644
--- a/include/iptables.h
+++ b/include/iptables.h
@@ -1,8 +1,9 @@
 #ifndef _IPTABLES_USER_H
 #define _IPTABLES_USER_H
 
-#include "xtables.h"
-#include "libiptc/libiptc.h"
+#include <netinet/ip.h>
+#include <xtables.h>
+#include <libiptc/libiptc.h>
 
 #ifndef IPT_SO_GET_REVISION_MATCH /* Old kernel source. */
 #define IPT_SO_GET_REVISION_MATCH	(IPT_BASE_CTL + 2)
diff --git a/include/libiptc/libxtc.h b/include/libiptc/libxtc.h
index 031afb5..3701018 100644
--- a/include/libiptc/libxtc.h
+++ b/include/libiptc/libxtc.h
@@ -20,8 +20,6 @@ extern "C" {
 #define XT_ALIGN(s) (((s) + ((XT_MIN_ALIGN)-1)) & ~((XT_MIN_ALIGN)-1))
 #endif
 
-typedef char xt_chainlabel[32];
-
 #define XTC_LABEL_ACCEPT  "ACCEPT"
 #define XTC_LABEL_DROP    "DROP"
 #define XTC_LABEL_QUEUE   "QUEUE"
diff --git a/include/xtables.h.in b/include/xtables.h.in
index 4ce73e9..51cb67d 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -1,11 +1,12 @@
 #ifndef _XTABLES_H
 #define _XTABLES_H
 
+#include <sys/socket.h> /* PF_* */
 #include <sys/types.h>
+#include <stdbool.h>
+#include <net/if.h>
 #include <linux/types.h>
 #include <linux/netfilter/x_tables.h>
-#include <libiptc/libxtc.h>
-#include <stdbool.h>
 
 #ifndef IPPROTO_SCTP
 #define IPPROTO_SCTP 132
@@ -22,12 +23,14 @@
 
 #define XTABLES_API_VERSION(x,y,z)    (0x10000*(x) + 0x100*(y) + z)
 
+struct in_addr;
+
 /* Include file for additions: new matches and targets. */
 struct xtables_match
 {
 	struct xtables_match *next;
 
-	xt_chainlabel name;
+	const char *name;
 
 	/* Revision of match (0 by default). */
 	u_int8_t revision;
@@ -83,7 +86,7 @@ struct xtables_target
 {
 	struct xtables_target *next;
 
-	xt_chainlabel name;
+	const char *name;
 
 	/* Revision of target (0 by default). */
 	u_int8_t revision;
diff --git a/xtables.c b/xtables.c
index a97bdaa..abdd283 100644
--- a/xtables.c
+++ b/xtables.c
@@ -32,6 +32,7 @@
 #include <arpa/inet.h>
 
 #include <xtables.h>
+#include <libiptc/libxtc.h>
 
 #ifndef NO_SHARED_LIBS
 #include <dlfcn.h>

Re: src: remove dependency on libiptc headers

[Pablo Neira Ayuso]

Jan Engelhardt wrote:
> --- a/include/libiptc/libxtc.h
> +++ b/include/libiptc/libxtc.h
> @@ -20,8 +20,6 @@ extern "C" {
>  #define XT_ALIGN(s) (((s) + ((XT_MIN_ALIGN)-1)) & ~((XT_MIN_ALIGN)-1))
>  #endif
>  
> -typedef char xt_chainlabel[32];
> -
>  #define XTC_LABEL_ACCEPT  "ACCEPT"
>  #define XTC_LABEL_DROP    "DROP"
>  #define XTC_LABEL_QUEUE   "QUEUE"
> diff --git a/include/xtables.h.in b/include/xtables.h.in
> index 4ce73e9..51cb67d 100644
> --- a/include/xtables.h.in
> +++ b/include/xtables.h.in
> @@ -1,11 +1,12 @@
>  #ifndef _XTABLES_H
>  #define _XTABLES_H
>  
> +#include <sys/socket.h> /* PF_* */
>  #include <sys/types.h>
> +#include <stdbool.h>
> +#include <net/if.h>
>  #include <linux/types.h>
>  #include <linux/netfilter/x_tables.h>
> -#include <libiptc/libxtc.h>
> -#include <stdbool.h>
>  
>  #ifndef IPPROTO_SCTP
>  #define IPPROTO_SCTP 132
> @@ -22,12 +23,14 @@
>  
>  #define XTABLES_API_VERSION(x,y,z)    (0x10000*(x) + 0x100*(y) + z)
>  
> +struct in_addr;
> +
>  /* Include file for additions: new matches and targets. */
>  struct xtables_match
>  {
>  	struct xtables_match *next;
>  
> -	xt_chainlabel name;
> +	const char *name;
>  
>  	/* Revision of match (0 by default). */
>  	u_int8_t revision;
> @@ -83,7 +86,7 @@ struct xtables_target
>  {
>  	struct xtables_target *next;
>  
> -	xt_chainlabel name;
> +	const char *name;

This is breaking the ABI. You're replacing at field of 32 bytes by one
of 4 bytes.

As we discussed in your previous patch, if your intention is to export
the xtables.h file and make it a public API for other external clients,
you must understand that you should not change a single line of headers
otherwise you may end up breaking backward compatibility (compilation
and binary) for others.

BTW, about the C++ support for xtables. Would we provide support to such
external plugin here? I think that we'll end up telling that we don't
recommend C++ plugins for iptables. I think that we should remove that
support now.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: src: remove dependency on libiptc headers

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> Jan Engelhardt wrote:
>> --- a/include/libiptc/libxtc.h
>> +++ b/include/libiptc/libxtc.h
>> @@ -20,8 +20,6 @@ extern "C" {
>>  #define XT_ALIGN(s) (((s) + ((XT_MIN_ALIGN)-1)) & ~((XT_MIN_ALIGN)-1))
>>  #endif
>>  
>> -typedef char xt_chainlabel[32];
>> -
>>  #define XTC_LABEL_ACCEPT  "ACCEPT"
>>  #define XTC_LABEL_DROP    "DROP"
>>  #define XTC_LABEL_QUEUE   "QUEUE"
>> diff --git a/include/xtables.h.in b/include/xtables.h.in
>> index 4ce73e9..51cb67d 100644
>> --- a/include/xtables.h.in
>> +++ b/include/xtables.h.in
>> @@ -1,11 +1,12 @@
>>  #ifndef _XTABLES_H
>>  #define _XTABLES_H
>>  
>> +#include <sys/socket.h> /* PF_* */
>>  #include <sys/types.h>
>> +#include <stdbool.h>
>> +#include <net/if.h>
>>  #include <linux/types.h>
>>  #include <linux/netfilter/x_tables.h>
>> -#include <libiptc/libxtc.h>
>> -#include <stdbool.h>
>>  
>>  #ifndef IPPROTO_SCTP
>>  #define IPPROTO_SCTP 132
>> @@ -22,12 +23,14 @@
>>  
>>  #define XTABLES_API_VERSION(x,y,z)    (0x10000*(x) + 0x100*(y) + z)
>>  
>> +struct in_addr;
>> +
>>  /* Include file for additions: new matches and targets. */
>>  struct xtables_match
>>  {
>>  	struct xtables_match *next;
>>  
>> -	xt_chainlabel name;
>> +	const char *name;
>>  
>>  	/* Revision of match (0 by default). */
>>  	u_int8_t revision;
>> @@ -83,7 +86,7 @@ struct xtables_target
>>  {
>>  	struct xtables_target *next;
>>  
>> -	xt_chainlabel name;
>> +	const char *name;
> 
> This is breaking the ABI. You're replacing at field of 32 bytes by one
> of 4 bytes.
> 
> As we discussed in your previous patch, if your intention is to export
> the xtables.h file and make it a public API for other external clients,
> you must understand that you should not change a single line of headers
> otherwise you may end up breaking backward compatibility (compilation
> and binary) for others.

Well, let's try to fix this and hope that nobody's is including libxtc.h
directly.

Please, resend the two patches using char name[32] instead of char *.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: src: remove dependency on libiptc headers

[Jan Engelhardt]

On Saturday 2008-08-02 07:52, Pablo Neira Ayuso wrote:
>>>  
#define XTABLES_VERSION "@PACKAGE_VERSION@"                                     
>>>  #define XTABLES_API_VERSION(x,y,z)    (0x10000*(x) + 0x100*(y) + z)
>>>  
>>> -	xt_chainlabel name;
>>> +	const char *name;
>>>  
>> This is breaking the ABI. You're replacing at field of 32 bytes by one
>> of 4 bytes.

The API actually remains the same, due to the nature of char[]
decaying into char* (and .name is also only used in the registrator);
the ABI (indicated by XTABLES_VERSION) changes yes, and that is fine
because we moved from 1.4.1 to 1.4.2-rc1.

Re: src: remove dependency on libiptc headers

[Patrick McHardy]

Jan Engelhardt wrote:
> src: remove dependency on libiptc headers
> 
> xtables.h does not need really need libxtc.h, and we can drop it from 
> the install as it is internal-only.

Applied, thanks.

Re: src: remove dependency on libiptc headers

[Pablo Neira Ayuso]

Jan Engelhardt wrote:
> On Saturday 2008-08-02 07:52, Pablo Neira Ayuso wrote:
>>>>  
> #define XTABLES_VERSION "@PACKAGE_VERSION@"                                     
>>>>  #define XTABLES_API_VERSION(x,y,z)    (0x10000*(x) + 0x100*(y) + z)
>>>>  
>>>> -	xt_chainlabel name;
>>>> +	const char *name;
>>>>  
>>> This is breaking the ABI. You're replacing at field of 32 bytes by one
>>> of 4 bytes.
> 
> The API actually remains the same, due to the nature of char[]
> decaying into char* (and .name is also only used in the registrator);
> the ABI (indicated by XTABLES_VERSION) changes yes, and that is fine
> because we moved from 1.4.1 to 1.4.2-rc1.

Sure, but say that someone uses whatever iproute2 version <=
iproute2-2.6.26 - which does not check for any ABI version numbering
AFAICS - and then it compiles whatever >= 1.4.2-rc1 by hand, then
jamal's ipt thing will not work as they use different ABIs.

I'm not talking on how to solve future interdependecy problems - which
you seems to have it done by exporting xtables as library - but the
problems that the size change of .name will trigger for some time.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: src: remove dependency on libiptc headers

[Jan Engelhardt]

On Monday 2008-08-04 10:35, Pablo Neira Ayuso wrote:
>> 
>> The API actually remains the same, due to the nature of char[]
>> decaying into char* (and .name is also only used in the registrator);
>> the ABI (indicated by XTABLES_VERSION) changes yes, and that is fine
>> because we moved from 1.4.1 to 1.4.2-rc1.
>
>Sure, but say that someone uses whatever iproute2 version <=
>iproute2-2.6.26 - which does not check for any ABI version numbering
>AFAICS - and then it compiles whatever >= 1.4.2-rc1 by hand, then
>jamal's ipt thing will not work as they use different ABIs.
>
>I'm not talking on how to solve future interdependecy problems - which
>you seems to have it done by exporting xtables as library - but the
>problems that the size change of .name will trigger for some time.

If the ABI changes, the libxtables.so ABI version number just get
bumped, that is what they are there for.

Re: src: remove dependency on libiptc headers

[Pablo Neira Ayuso]

Jan Engelhardt wrote:
> On Monday 2008-08-04 10:35, Pablo Neira Ayuso wrote:
>>> The API actually remains the same, due to the nature of char[]
>>> decaying into char* (and .name is also only used in the registrator);
>>> the ABI (indicated by XTABLES_VERSION) changes yes, and that is fine
>>> because we moved from 1.4.1 to 1.4.2-rc1.
>> Sure, but say that someone uses whatever iproute2 version <=
>> iproute2-2.6.26 - which does not check for any ABI version numbering
>> AFAICS - and then it compiles whatever >= 1.4.2-rc1 by hand, then
>> jamal's ipt thing will not work as they use different ABIs.
>>
>> I'm not talking on how to solve future interdependecy problems - which
>> you seems to have it done by exporting xtables as library - but the
>> problems that the size change of .name will trigger for some time.
> 
> If the ABI changes, the libxtables.so ABI version number just get
> bumped, that is what they are there for.

Great. You're breaking previous backward compatibility with old iproute2
versions since they are not checking that number.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: src: remove dependency on libiptc headers

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Jan Engelhardt wrote:
>> On Monday 2008-08-04 10:35, Pablo Neira Ayuso wrote:
>>>> The API actually remains the same, due to the nature of char[]
>>>> decaying into char* (and .name is also only used in the registrator);
>>>> the ABI (indicated by XTABLES_VERSION) changes yes, and that is fine
>>>> because we moved from 1.4.1 to 1.4.2-rc1.
>>> Sure, but say that someone uses whatever iproute2 version <=
>>> iproute2-2.6.26 - which does not check for any ABI version numbering
>>> AFAICS - and then it compiles whatever >= 1.4.2-rc1 by hand, then
>>> jamal's ipt thing will not work as they use different ABIs.
>>>
>>> I'm not talking on how to solve future interdependecy problems - which
>>> you seems to have it done by exporting xtables as library - but the
>>> problems that the size change of .name will trigger for some time.
>> If the ABI changes, the libxtables.so ABI version number just get
>> bumped, that is what they are there for.
> 
> Great. You're breaking previous backward compatibility with old iproute2
> versions since they are not checking that number.

Jumping in late, but I already applied that patch. The iproute
compabibility is already broken regulary because the "interface"
is a huge hack and it duplicates all kinds of internal functions.
So this doesn't really make things worse. I don't think its
reasonable to not change internal functions/structs because
something external is dlopening and fiddling in iptables internals.
Once iproute makes use of libxtables I'm willing to be more
careful about this.