So, is this patch being rejected then?
-Brad
On 9/1/16 6:54 PM, Brad House via c-ares wrote:
> Hi Daniel,
>
> Based on your reply, I'm thinking you maybe didn't realize I attached a
> patch on the original email :)
>
> Most of the attached patch is just removal of unused defines, so if you
> did see it, maybe you thought I just removed the defines rather than
> fixing issues where they existed. But it does fix last couple of usages
> of those configure defines as well ... I think there must have been a
> code cleanup at some point of c-ares to get rid of 99% of the usages
> as otherwise I can't explain their existence at all.
>
> I also think it is silly to reconfigure and build for each arch independently
> when using XCode on Apple. That's not how anyone builds software on Apple
> since you'd then have to manually re-merge the various architectures into a
> single binary using "lipo" ... not a fun process. This patch solves that
> issue on both iOS and MacOS.
>
> I should have mentioned this in the original email, I did test the patch
> on just about every system I have access to ... I doubt most patches see
> this set of platform tests :)
>
>
> - Linux x86
> - Linux x64
> - Windows x86 - Visual Studio 2005
> - Windows x64 - Visual Studio 2005
> - Windows x86 - MinGW-w64
> - Windows x64 - MinGW-w64
> - AIX 32bit GCC
> - AIX 64bit GCC
> - Solaris Sparc 9/10 32bit GCC
> - Solaris Sparc 9/10 64bit GCC
> - Solaris 10 x86 32bit
> - Solaris 10 x64 64bit
> - FreeBSD 9/10 x86
> - FreeBSD 9/10 x64
> - MacOSX x86
> - MacOSX x64
> - iOS arm 32bit
> - iOS arm 64bit
> - Android arm 32bit
> - SCO OpenServer 5
> - SCO OpenServer 6
> - Linux - Raspbian arm
>
>
> On 9/1/16 5:39 PM, Daniel Stenberg wrote:
>> On Thu, 1 Sep 2016, Brad House via c-ares wrote:
>>
>>> Configure-time type size checks are a really bad idea, especially for things like Apple (MacOS, iOS) where you could
>>> be building multi-arch, where you run configure once, but the compiler, internally, is building the source more than
>>> once (per architecture).
>>
>> It would be nice to get rid of those, yes.
>>
>> The problem isn't the configure checks really, but if some code really needs the sizes there's really no good way around
>> the checks. Ideally we should of course write code that relies as little as possible on such.
>>
>>> That means if you hard-code SIZEOF_LONG 4 based on what configure detected, but building arm32 and arm64
>>> simultaneously, you get a misbuild on at least one of the platforms!
>>
>> For such platforms - if the size checks are really necessary - configure should be run twice. Once for each architecture
>> and then two separate builds.
>>
>>> Its a fairly trivial patch to remove these dependencies on detected preprocessor
>>> macros, for the most part they aren't used, except for a couple of cases.
>>
>> So how can you remove them for the cases where they are needed?
>>
>> Also, this little changelog note from Jan 14 2009 seems relevant (I didn't really get into the exact details, maybe ẃe
>> can still just work around it).
>>
>> - ares.h no longer uses the HAVE_STRUCT_IN6_ADDR define check, but instead it
>> now declares the private struct ares_in6_addr for all systems instead of
>> relying on one possibly not present in the system.
>>
>>> Any preprocessor macros provided by the OS for similar purposes are considered to be safe.
>>
>> If they exist for the OSes/platforms, sure. c-ares has after all been compiled and run on a really vast number of
>> platforms so it is a bit of a fragile area to just rip out large pieces from. It needs careful attention to details.
>>
>
Received on 2016-09-12