Re: [Mingw-w64-public] Clean build of mingw-w64 (ie no more warning)

[dw]

On 8/14/2016 9:23 AM, NightStrike wrote:
> On Sun, Aug 14, 2016 at 9:02 AM, Martin Storsjö  wrote:
>> On Sat, 13 Aug 2016, dw wrote:
>>
>>> I still have some more fixes for ARM, but this patch is getting too 
>>> big.
>>> This is a logical point to break.
>> Yes, that's probably for the best.
>>
>> If/when committing (iirc someone had already ok'd it?),
> Who ok'd it?

Yeah, I'm pretty sure no one has yet.

>> I think it'd be
>> even better to split it further, to one commit per issue.
>> There also seems to be a few other changes in the patch not directly
>> related to getting rid of warnings:
>> - gs_support.c, only whitespace change in UNW_FLAG_NHANDLER, nothing 
>> else
>> changed on that line?

Actually, there are 2 changes on that line.  One is spacing, the other 
is changing "0x00" to "0x0".  However there are other changes in that 
patch that *are* just spacing (see basetyps.h).

>> - _vswprintf_p.c, _vscwprintf_p.c - these also add a comment that wasn't
>> there before. Probably ok, but I guess it's preferrable to have such
>> changes split out.
>> - aviriff.h, I see no other changes than adding in leading zeros

Yes, that's true.  However, there is another header that *doesn't* have 
those zeros.  So while from a functional point of view it makes 
absolutely no difference, the compiler warns about the "redefinition."  
Even a difference in spacing triggers this warning.

A lot of the changes here are just like this.  Trivial, nothing changes, 
that nonetheless trigger warnings.  That's the only reason I'm prepared 
to make a patch this big.

>> - basetyps.h, also only fixing whitespace?

Yup.

>> - mfidl.h, changing hex constants from upper case to lower?

Yeah.

>> - winnt.h, removing leading zeros in hex constants?

Uh huh.

>> So I think it'd be better to commit the fixes for each issue (not per
>> file, but per issue) separately, with an explanation on what 
>> warning/issue
>> it fixes, or why it stylistically is preferrable. (E.g. the list of 
>> files
>> and changes you have only mention "redefine errors" for winnt.h.)
> Yes, definitely split it up into smaller commits as described here.

In my mind, the "issue" I was resolving was "cleaning up warnings." 
Every line in this patch was designed with that goal (well, with the 
exception of the copyright.  That's just habit). I didn't set out to 
"fix redefine errors," then "fix unreferenced parameter errors," etc.  
So in the end, I just have a bunch of files with fixes.

While I can split this up into smaller patches if that's what it takes 
to get it approved, I'm not looking forward to it.  At ~one patch per 
file, that's ~30 patches.  If I compile each one before I check it in to 
make sure it can stand alone, that's what, 6 hours of compiling?  Ouch.  
I don't know if people like to build patches before they approve them, 
but 30 of these would be a huge hassle for the approvers as well.

But since Nightstrike and Martin want this broken up before they approve 
it, how about grouping them like this:

= Missing prototypes =
clog10.c, clog10f.c, clog10l.c:
   - The prototypes for the functions created in these files are
 protected by #ifdef _GNU_SOURCE.  Since we should always build
 the .c file, we need to add the define so we can find the
 prototype.
gai_strerrorA.c:
   - wcstombs() is in stdlib.
abs64.c:
   - llabs() is in stdlib.h.
_vscprintf_p.c, _vscwprintf_p.c, _vswprintf_p.c:
   - _vscprintf_p_l, _vscwprintf_p_l & _vswprintf_p_l prototypes are
 protected by #if.  Add the define so we can find them.

= #define mismatches =
gs_support.c:
   - The minor variation causes a 'redefines' error.
cephes_emath.h:
   - The minor variations cause 'redefines' errors.
uchar.h:
   - __STDC_UTF_16__ & __STDC_UTF_32__ are defined by the gcc
 compiler.
dxgi.h:
   - Cheap way to avoid 'redefine' errors.
aviriff.h, basetyps.h, combaseapi.h, gpedit.h:
   - Redefine errors.
mfapi.h:
   - Redefine errors.
mfidl.h:
   - Redefine errors.
ntdef.h:
   - Redefine errors.
ntsecapi.h:
   - These redefine values that are unconditionally defined earlier
 in the same file.
winnt.h:
   - Redefine errors.

= Unreferenced variables =
gs_support.c:
   - ARM does not use StackCookie.  I'd like to use the already-
 defined __UNUSED_PARAM from _mingw.h, but it uses an attribute,
 so I'd have to put it on the function declaration, and then I'd
 have to #ifdef it by platform.  Yuck.
ftw.c:
   - Unused parameters.

= Add Pragma Diagnostic =
tdelete.c:
   - Comparing NULL to function pointer generates warning.
stdio.h:
   - We are shadowing a number of functions provided by the compiler
(snprintf, vsnprintf, etc).  Ignore warnings.
mfapi.h:
   - FCC ('AI44') et al cause 'multichar' compiler warnings. Ignore
 them.

= Misc warnings =
e_pow.c:
   - Sign mismatch in comparison.

Re: [Mingw-w64-public] Clean build of mingw-w64 (ie no more warning)

[Martin Storsjö]

On Sun, 14 Aug 2016, NightStrike wrote:


On Sun, Aug 14, 2016 at 9:02 AM, Martin Storsjö  wrote:

On Sat, 13 Aug 2016, dw wrote:


I still have some more fixes for ARM, but this patch is getting too big.
This is a logical point to break.


Yes, that's probably for the best.

If/when committing (iirc someone had already ok'd it?),


Who ok'd it?


Ah, apparently I misremembered - nevermind then.

// Martin--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Re: [Mingw-w64-public] Clean build of mingw-w64 (ie no more warning)

[NightStrike]

On Sun, Aug 14, 2016 at 9:02 AM, Martin Storsjö  wrote:
> On Sat, 13 Aug 2016, dw wrote:
>
>> I still have some more fixes for ARM, but this patch is getting too big.
>> This is a logical point to break.
>
> Yes, that's probably for the best.
>
> If/when committing (iirc someone had already ok'd it?),

Who ok'd it?

> I think it'd be
> even better to split it further, to one commit per issue.

> There also seems to be a few other changes in the patch not directly
> related to getting rid of warnings:
> - gs_support.c, only whitespace change in UNW_FLAG_NHANDLER, nothing else
>changed on that line?
> - _vswprintf_p.c, _vscwprintf_p.c - these also add a comment that wasn't
>there before. Probably ok, but I guess it's preferrable to have such
>changes split out.
> - aviriff.h, I see no other changes than adding in leading zeros
> - basetyps.h, also only fixing whitespace?
> - mfidl.h, changing hex constants from upper case to lower?
> - winnt.h, removing leading zeros in hex constants?
>
> So I think it'd be better to commit the fixes for each issue (not per
> file, but per issue) separately, with an explanation on what warning/issue
> it fixes, or why it stylistically is preferrable. (E.g. the list of files
> and changes you have only mention "redefine errors" for winnt.h.)

Yes, definitely split it up into smaller commits as described here.

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Re: [Mingw-w64-public] Clean build of mingw-w64 (ie no more warning)

[Martin Storsjö]

On Sat, 13 Aug 2016, dw wrote:

> I still have some more fixes for ARM, but this patch is getting too big. 
> This is a logical point to break.

Yes, that's probably for the best.

If/when committing (iirc someone had already ok'd it?), I think it'd be 
even better to split it further, to one commit per issue.

There also seems to be a few other changes in the patch not directly 
related to getting rid of warnings:
- gs_support.c, only whitespace change in UNW_FLAG_NHANDLER, nothing else
   changed on that line?
- _vswprintf_p.c, _vscwprintf_p.c - these also add a comment that wasn't
   there before. Probably ok, but I guess it's preferrable to have such
   changes split out.
- aviriff.h, I see no other changes than adding in leading zeros
- basetyps.h, also only fixing whitespace?
- mfidl.h, changing hex constants from upper case to lower?
- winnt.h, removing leading zeros in hex constants?

So I think it'd be better to commit the fixes for each issue (not per 
file, but per issue) separately, with an explanation on what warning/issue 
it fixes, or why it stylistically is preferrable. (E.g. the list of files 
and changes you have only mention "redefine errors" for winnt.h.)

// Martin

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public