[Olsr-dev] [PATCH v1 1/2] android: fix type punning warnings on IN6_IS_ADDR_V4MAPPED

[Hans-Christoph Steiner]

On 10/22/2012 12:17 PM, Ferry Huberts wrote:
> 
> 
> On 22-10-12 18:09, Hans-Christoph Steiner wrote:
>>
>>
>> On 10/22/2012 11:45 AM, Ferry Huberts wrote:
>>>
>>>
>>> On 22-10-12 17:28, Hans-Christoph Steiner wrote:
>>>>
>>>>
>>>> On 10/22/2012 04:59 AM, Ferry Huberts wrote:
>>>>>
>>>>>
>>>>> On 19-10-12 18:12, Hans-Christoph Steiner wrote:
>>>>>>
>>>>>> Android is not GNU and its not UNIX, so including a header from GNU seems
>>>>>> like
>>>>>> a bad idea.  Android does not use GNU libc nor does it use BSD libc.  It
>>>>>> uses
>>>>>> bionic libc.  Yes, Android uses the Linux kernel, but these headers are not
>>>>>> from the Linux kernel, they are related to libc.
>>>>>>
>>>>>> I think we'll have to do something ugly anyhow, unfortunately.  Maybe for
>>>>>> android, the v6 part of p->prefix.v6 will need to be defined the same way
>>>>>> that
>>>>>> its done in the Android in6.h header and wrapped in a #ifdef __ANDROID__
>>>>>> #else
>>>>>>
>>>>>
>>>>> I see no problem in doing it this way.
>>>>> However, if you want it different, then can you please show us that
>>>>> change/patch?
>>>>>
>>>>> We're almost at the point of zero warnings after we fix this one, so why
>>>>> stop
>>>>> now?
>>>>
>>>>
>>>> Because preventing bugs is far more important than compiler warnings.
>>>> Compiler warnings are a useful tool, but never a goal.  Using the wrong
>>>> headers is a recipe for disaster, especially when the whole problem you are
>>>> trying to solve comes from differences in GNU libc and Android's bionic libc.
>>>>
>>>
>>>
>>> I'll ask again: if you want it different, then can you please show us that
>>> change/patch?
>>>
>>> You appointed yourself olsrd Android maintainer, so please fix the warnings.
>>> Fix them in a way that is acceptable to you, and to us please.
>>
>> This sounds to me like kind of a threat, did you mean for it to sound like
>> that?  Tone in email is often misconstrued, so I thought I'd ask.
>>
> 
> Ofcourse not.
> I said please :-)
> 
> We want to move into a position so that we can enable more restrictive
> compiler flags. These warnings are holding that back.
> 
> As you can see in the repo, we also have Coverity builds now.
> It's all part of the plan to have a very robust and secure olsrd.
> 
> If you want I can give you some Coverity warning logs on jsoninfo...

Yes please, I'm interested in seeing coverity logs from all of olsrd.


>> My wife is going to give birth any day now, I'll see what I can do, but its
>> unlikely to happen any time soon.  Once the baby comes, I'm out for 6 weeks.
>>
>> I think the approach with using #ifdef __ANDROID__ in the definition of
>> p->prefix.v6 will work, but I'm open to ideas on how to make it cleaner.  I'm
>> also OK with ignoring the warning if the current code is functional.  I'm not
>> sure anyone is really testing olsrd with IPv6 on Android.
>>
> 
> Please whip up a patch then before the baby comes :-)

If it was that simple, it would be done.  So far I've gotten no feedback on
the idea I outlined above.  Personally, I'd leave the code that's generating
this warning on Android as is until someone is actually setup to test things
using IPv6 on Android.  Commotion is currently all IPv4.  I've never used IPv6
and its not part of my current mandate for olsrd work, which I'm scrambling to
get done before the baby comes.

.hc

> 
>> .hc
>>
>>
>>>> .hc
>>>>
>>>>
>>>>>
>>>>>
>>>>>> .hc
>>>>>>
>>>>>> On 10/19/2012 12:05 PM, Ferry Huberts wrote:
>>>>>>> From: Ferry Huberts <(spam-protected)>
>>>>>>>
>>>>>>> Signed-off-by: Ferry Huberts <(spam-protected)>
>>>>>>> ---
>>>>>>>     android/linux/olsr_netinet_in.h | 52
>>>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>>>     src/ipcalc.h                    |  4 ++++
>>>>>>>     2 files changed, 56 insertions(+)
>>>>>>>     create mode 100644 android/linux/olsr_netinet_in.h
>>>>>>>
>>>>>>> diff --git a/android/linux/olsr_netinet_in.h
>>>>>>> b/android/linux/olsr_netinet_in.h
>>>>>>> new file mode 100644
>>>>>>> index 0000000..7500c80
>>>>>>> --- /dev/null
>>>>>>> +++ b/android/linux/olsr_netinet_in.h
>>>>>>> @@ -0,0 +1,52 @@
>>>>>>> +/* Copyright (C) 1991-2001, 2003, 2004, 2006, 2007, 2008, 2011
>>>>>>> +   Free Software Foundation, Inc.
>>>>>>> +   This file is part of the GNU C Library.
>>>>>>> +
>>>>>>> +   The GNU C Library is free software; you can redistribute it and/or
>>>>>>> +   modify it under the terms of the GNU Lesser General Public
>>>>>>> +   License as published by the Free Software Foundation; either
>>>>>>> +   version 2.1 of the License, or (at your option) any later version.
>>>>>>> +
>>>>>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>>>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>>>>> +   Lesser General Public License for more details.
>>>>>>> +
>>>>>>> +   You should have received a copy of the GNU Lesser General Public
>>>>>>> +   License along with the GNU C Library; if not, write to the Free
>>>>>>> +   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>>>>>>> +   02111-1307 USA.
>>>>>>> +
>>>>>>> +
>>>>>>> +   This is a stripped copy of netinet/in.h that includes only the
>>>>>>> +   IN6_IS_ADDR_V4MAPPED define. It is used to fix warnings on the
>>>>>>> +   Android build.
>>>>>>> +*/
>>>>>>> +
>>>>>>> +#ifndef    _OLSR_NETINET_IN_H
>>>>>>> +#define    _OLSR_NETINET_IN_H    1
>>>>>>> +
>>>>>>> +#ifdef IN6_IS_ADDR_V4MAPPED
>>>>>>> +  #undef IN6_IS_ADDR_V4MAPPED
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +
>>>>>>> +#ifdef __GNUC__
>>>>>>> +
>>>>>>> +# define IN6_IS_ADDR_V4MAPPED(a) \
>>>>>>> +  (__extension__                                  \
>>>>>>> +   ({ __const struct in6_addr *__a = (__const struct in6_addr *)
>>>>>>> (a);          \
>>>>>>> +      __a->s6_addr32[0] == 0                              \
>>>>>>> +      && __a->s6_addr32[1] == 0                              \
>>>>>>> +      && __a->s6_addr32[2] == htonl (0xffff); }))
>>>>>>> +
>>>>>>> +#else
>>>>>>> +
>>>>>>> +# define IN6_IS_ADDR_V4MAPPED(a) \
>>>>>>> +    ((((__const uint32_t *) (a))[0] == 0)                      \
>>>>>>> +     && (((__const uint32_t *) (a))[1] == 0)                  \
>>>>>>> +     && (((__const uint32_t *) (a))[2] == htonl (0xffff)))
>>>>>>> +
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +#endif    /* _OLSR_NETINET_IN_H */
>>>>>>> diff --git a/src/ipcalc.h b/src/ipcalc.h
>>>>>>> index 30b8068..4dde3f4 100644
>>>>>>> --- a/src/ipcalc.h
>>>>>>> +++ b/src/ipcalc.h
>>>>>>> @@ -49,6 +49,10 @@
>>>>>>>     #include <arpa/inet.h>
>>>>>>>     #include <netinet/in.h>
>>>>>>>
>>>>>>> +#ifdef __ANDROID__
>>>>>>> +  #include <linux/olsr_netinet_in.h>
>>>>>>> +#endif
>>>>>>> +
>>>>>>>     extern const struct olsr_ip_prefix ipv4_internet_route;
>>>>>>>     extern const struct olsr_ip_prefix ipv6_mappedv4_route;
>>>>>>>     extern const struct olsr_ip_prefix ipv6_internet_route;
>>>>>>>
>>>>>
>>>
>