On 2013.02.16 17:50, Ludovic Rousseau wrote:
> I installed MinGW on Debian. And the compiler reported warnings and
> potential bugs.
Because you're using pedantic options, right?
Can you provide the extra compiler options are you using?
> I fixed most of the warnings. See the pull request [1].
Great, and those warnings look OK to me.
But as I stated previously, I'm not going to test/commit them unless
they are proposed as a single patch (or at best 2 patches), because I
really don't see how pushing a bunch of one-liners that pertain to
fixing a set of non-risky warnings, when a single commit could be used,
is going to make my and the life of our users better. On the contrary, I
think that anybody looking at our commits who has to churn through a set
of 6 warning fixes when dealing with the libusbx tree, only to find out
that they are utterly benign and are limited to a couple of lines each,
is going be more annoyed than anything else.
Of course, you're free to push them yourself if you feel like it.
> The remaining warnings are:
> ../../libusb/os/windows_usb.c: In function 'windows_init':
> ../../libusb/os/windows_usb.c:879:18: warning: cast from function call
> of type 'uintptr_t' to non-matching type 'void *'
> [-Wbad-function-cast]
What's the point of using uintptr_t if you can't cast to and from void*?
My understanding is that uintprt_t was introduced to be able to do
pointer arithmetic, so casts from uintptr_t to void* should never
produce a warning.
We're just casting the return value of _beginthreadex, which is an
uintptr_t to a HANDLE, which is typedef'd as void*, because we need to
pass that parameter as a HANDLE later on. No idea how to fix that, and
I'm really tempted to say don't care, because the compiler shouldn't
bother us here.
> The line
> SP_DEVINFO_DATA dev_info_data = { 0 };
> may be missing some initializers
Most compilers should understand that line as zeroing the whole
structure. Or maybe gcc wants {} rather than { 0 }? Only problem with
that is that MSVC doesn't take {}.
I'd hate to have to do a memset, or stuff the whole init just for
something that any compiler worth its name should understand the meaning of.
> ../../libusb/os/windows_usb.c: In function 'winusbx_claim_interface':
> ../../libusb/os/windows_usb.c:376:30: warning: 'dev_info' may be used
> uninitialized in this function [-Wuninitialized]
> ../../libusb/os/windows_usb.c:2672:11: note: 'dev_info' was declared here
>
> This may be a real bug.
> dev_info is set only if (_index <= 0) in the previous lines.
> Maybe a else statement is missing?
I'll try to look into that. No idea when.
>
> ../../libusb/os/threads_windows.c:137:1: warning: '__inline' is not at
> beginning of declaration [-Wold-style-declaration]
The compiler's probably right on that one. Easy to fix.
>
> ../../libusb/os/threads_windows.c: In function 'usbi_cond_timedwait':
> ../../libusb/os/threads_windows.c:190:2: warning: nested extern
> declaration of 'epoch_time' [-Wnested-externs]
Might have to add include guards somewhere, unless it's a MinGW issue.
Regards,
/Pete
------------------------------------------------------------------------------
The Go Parallel Website, sponsored by Intel - in partnership with Geeknet,
is your hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials, tech docs,
whitepapers, evaluation guides, and opinion stories. Check out the most
recent posts - join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel
