On 8/14/2016 9:23 AM, NightStrike wrote:
> On Sun, Aug 14, 2016 at 9:02 AM, Martin Storsjö <[email protected]> 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.
mingw_pformat.c:
- ARM does not support __attribute__((gcc_struct))
mingw_vfscanf.c:
- Fix incorrect indention.
mftransform.h:
- EXTERN_C is either defined as 'extern' or 'extern "C"'. However,
this isn't really an extern (implying that the actual definition
is elsewhere), since the code also initializes the variable. And
we don't need 'extern "C"' since this entire section is already
wrapped with one. Using both 'extern' and initializing generates
a warning.
Would that suffice? I'd hate to take the time to create/test all of
these only to be told they need to be broken down even further.
dw
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
