ip(6)_tables.h: return type difference in ip(6)t_get_target

ip(6)_tables.h: return type difference in ip(6)t_get_target

[Henning Peters]

Hi netfilter-folks,

this is my first post, so I would like to say hello to all of you.
Netfilter/iptables is a nice piece of software, thank you for that.

Ok, but fortunately that's not the only reason why I post to this list.

I am working on a network protocol implementation which interacts with
middleboxes in general, NATs in particular. Harald does not believe that
this will be popular
(http://www.netfilter.org/documentation/conferences/nf-workshop-2004-summary.html#AEN128)...
and there is probably a big chance that he will be right.

This software is being written in c++. I want to avoid using the
command-line to talk to iptables for efficiency. Including iptables
headers and linking to libiptables seemed reasonable for me. But compiling
with g++ (gcc version 3.3.4 (pre 3.3.5 20040809)) and linking to the
c-libs (extern "C" stuff) gave me a compile error:

/usr/include/linux/netfilter_ipv6/ip6_tables.h:301: error: invalid
conversion from `void*' to `ip6t_entry_target*'

btw: the code for ipv4 and ipv6 is quite the same at this location:

/* Helper functions */
static __inline__ struct ip6t_entry_target *
ip6t_get_target(struct ip6t_entry *e)
{
        return (void *)e + e->target_offset;
}

My impression is that there don't have to be a void pointer in the return
statement. The gnu c++ compiler seems to be more picky than the c compiler
about the difference of declared return type and actual returned type.
Exchanging the (void *) with (struct ip6t_entry *) worked for me well. The
same goes for ipv4.

Can this be changed or is there a reason for it beeing the way how it is
currently in the code? If keeping the void pointer as in the return
statement is a concern, one could simply make the declared return type
fit.

Henning


These are my proposed changes:

netfilter_ipv6/ip6_tables.h
301c301
<       return (void *)e + e->target_offset;
---
>       return (struct ip6t_entry_target *)e + e->target_offset;

netfilter_ipv4/ip_tables.h
295c295
<       return (void *)e + e->target_offset;
---
>       return (struct ipt_entry_target *)e + e->target_offset;

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Pablo Neira]

Henning Peters wrote:
> This software is being written in c++. I want to avoid using the
> command-line to talk to iptables for efficiency. Including iptables
> headers and linking to libiptables seemed reasonable for me. But compiling
> with g++ (gcc version 3.3.4 (pre 3.3.5 20040809)) and linking to the
> c-libs (extern "C" stuff) gave me a compile error:

Two comments:

a) The internal library used by iptables (libiptc) is undocumented and
its use is discouraged.
b) iptables is entirely written in C, linking libiptc to C++ code isn't
a good a idea.

So, you should call iptables via system(). It isn't great but you'll
have less problems.

--
Pablo

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Henning Peters]

Hi Pablo,

as I said in my previous mail I am linking to libiptables and not to
libiptc. I was seeing libiptables API as a more higher level interface and
therefore I was choosing this. I think we agree in this point.

Still, both API/libs seem to be undocumented, but at least they are
advertised on netfilter.org: "multiple layers of API's for 3rd party
extensions".

> b) iptables is entirely written in C, linking libiptc to C++ code isn't
a good a idea.

Recently I have used a C++ wrapper for the OpenGL library which is also
written entirely in C. This gave me a quite robust impression although it
is intermixed code. The same goes for several other OOP wrappers which I
don't have experience in, but have heard about it. Technically, the C code
is not translated by the C++ compiler (like some other guys tried on this
list), the C++ compiler just knows the calling conventions of compiled C
code, is this a bad idea?

My impression is that if it is a regular API/library (as it is the case in
libiptables), then I don't see arguments not to use it in C++ software if
the alternatives are resource consuming and error prone. I see the
intermix features of C++ as a feature, why don't use it?

Maybe I am not aware of certain dangers in doing so. If you think this is
the case, please give me more details on this issue.

Back to my original post: Is there a reason not to change the code as I
proposed? I think this is a syntax error anyway so we don't have to waste
time on a C vs. C++ discussion.

Thanks,
Henning


Pablo Neira sagte Fr, 9.09.2005, 00:54:
> Henning Peters wrote:
>> This software is being written in c++. I want to avoid using the
command-line to talk to iptables for efficiency. Including iptables
headers and linking to libiptables seemed reasonable for me. But
compiling
>> with g++ (gcc version 3.3.4 (pre 3.3.5 20040809)) and linking to the
c-libs (extern "C" stuff) gave me a compile error:
>
> Two comments:
>
> a) The internal library used by iptables (libiptc) is undocumented and
its use is discouraged.
> b) iptables is entirely written in C, linking libiptc to C++ code isn't
a good a idea.
>
> So, you should call iptables via system(). It isn't great but you'll
have less problems.
>
> --
> Pablo

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Patrick Schaaf]

> Back to my original post: Is there a reason not to change the code as I
> proposed?

Your code change is wrong. See below for proof code. Remember that
a "pointer + integer" always advances the pointer according to the
type of the thing pointed to, i.e. by "integer*sizeof(*pointer)".
And casting seems to have precedence over "+"...

best regards
  Patrick

$ cat foo.c
struct foo { int a; };

int main(int argc, char **argv)
{
        struct foo fooarray[10];
        struct foo *bar = fooarray;
        void *baz = fooarray;

        printf("struct foo *: %p %p\n", bar, bar + 1);
        printf("void *: %p %p\n", baz, baz + 1);
        return 0;
}
$ gcc -o foo foo.c
$ ./foo
struct foo *: 0xbffff878 0xbffff87c
void *: 0xbffff878 0xbffff879

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Patrick Schaaf]

>> Back to my original post: Is there a reason not to change the code as I
>> proposed?
> 
> Your code change is wrong. See below for proof code. Remember that
> a "pointer + integer" always advances the pointer according to the
> type of the thing pointed to, i.e. by "integer*sizeof(*pointer)".
> And casting seems to have precedence over "+"...

Damn, my proof code was nonsensical in that it not tested for the
"casting seems to" property. Corrected code below. So, casting
has higher precedence than "+". Which does not make your proposed
change more correct, it's just wrong in a different way :)

best regards
  Patrick

$ cat foo.c
struct foo { int a; };
struct bar { int a; int b; };

int main(int argc, char **argv)
{
        struct foo fooarray[10];
        struct foo *bar = fooarray;

        printf("struct foo *: %p %p\n", bar, bar + 1);
        printf("struct bar *: %p %p\n", bar, (struct bar *) bar + 1);
        printf("void *: %p %p\n", bar, (void *) bar + 1);
        return 0;
}
$ gcc -o foo foo.c
$ ./foo
struct foo *: 0xbffff878 0xbffff87c
struct bar *: 0xbffff878 0xbffff880
void *: 0xbffff878 0xbffff879

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Henning Peters]

Hi Patrick,
yes, you are right. Thank you for the pointer and example code.

return (struct ip6t_entry_target *) ((void *)e + e->target_offset);

seems ugly, but necessary if compilers are more strict to match the return
type. I would appreciate if this could be applied in order to faciliate
C++/iptables development. I am not sure whether other C compilers might
not see this issue aswell?!

Henning


Patrick Schaaf sagte Fr, 9.09.2005, 11:03:
>>> Back to my original post: Is there a reason not to change the code as I
>>> proposed?
>>
>> Your code change is wrong. See below for proof code. Remember that
>> a "pointer + integer" always advances the pointer according to the
>> type of the thing pointed to, i.e. by "integer*sizeof(*pointer)".
>> And casting seems to have precedence over "+"...
>
> Damn, my proof code was nonsensical in that it not tested for the
> "casting seems to" property. Corrected code below. So, casting
> has higher precedence than "+". Which does not make your proposed
> change more correct, it's just wrong in a different way :)
>
> best regards
>   Patrick
>
> $ cat foo.c
> struct foo { int a; };
> struct bar { int a; int b; };
>
> int main(int argc, char **argv)
> {
>         struct foo fooarray[10];
>         struct foo *bar = fooarray;
>
>         printf("struct foo *: %p %p\n", bar, bar + 1);
>         printf("struct bar *: %p %p\n", bar, (struct bar *) bar + 1);
>         printf("void *: %p %p\n", bar, (void *) bar + 1);
>         return 0;
> }
> $ gcc -o foo foo.c
> $ ./foo
> struct foo *: 0xbffff878 0xbffff87c
> struct bar *: 0xbffff878 0xbffff880
> void *: 0xbffff878 0xbffff879
>
>

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Patrick Schaaf]

> return (struct ip6t_entry_target *) ((void *)e + e->target_offset);

I'd write this as

return (struct ip6t_entry_target *) (((char *)e) + e->target_offset);

Two reasons:

First, cast to void* is a gcc C (and maybe C++, dunno) extension.
If you have to cast the result to the correct type, anyway, you
can as well use "char *".

Second, by putting "(char *)e" in an additional set of braces,
the reader of the code need not think about precedence of
cast vs. addition, i.e. the meaning of the construct is
more obvious.

best regards
  Patrick

Re: ip(6)_tables.h: return type difference in ip(6)t_get_target

[Henning Peters]

>> return (struct ip6t_entry_target *) ((void *)e + e->target_offset);
>
> I'd write this as
>
> return (struct ip6t_entry_target *) (((char *)e) + e->target_offset);

I agree.

Henning