2013/2/17 Pete Batard <p...@akeo.ie>:
> 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?
No. Not yet :-)
> Can you provide the extra compiler options are you using?
$ echo $CFLAGS
-Wall -g -O2 -Wextra -pipe -funsigned-char -fstrict-aliasing
-Wchar-subscripts -Wundef -Wshadow -Wcast-align -Wwrite-strings
-Wunused -Wuninitialized -Wpointer-arith -Wredundant-decls -Winline
-Wformat -Wformat-security -Wswitch-enum -Winit-self
-Wmissing-include-dirs -Wmissing-prototypes -Wstrict-prototypes
-Wold-style-definition -Wbad-function-cast -Wnested-externs
-Wmissing-declarations
>> 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.
Ok. Pushed now.
>> 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.
You should have a value for _each_ field to initialize.
A correct patch is something like:
--- /tmp/RSwSqr_windows_usb.c 2013-02-18 18:58:42.258718606 +0100
+++ libusb/os/windows_usb.c 2013-02-18 18:58:23.000000000 +0100
@@ -1315,7 +1315,7 @@ static int windows_get_device_list(struc
struct discovered_devs *discdevs;
HDEVINFO dev_info = { 0 };
const char* usb_class[] = {"USB", "NUSB3", "IUSB3"};
- SP_DEVINFO_DATA dev_info_data = { 0 };
+ SP_DEVINFO_DATA dev_info_data = { 0, {0, 0, 0, {0, 0, 0, 0, 0, 0,
0, 0}}, 0, 0 };
SP_DEVICE_INTERFACE_DETAIL_DATA_A *dev_interface_details = NULL;
GUID hid_guid;
#define MAX_ENUM_GUIDS 64
Maybe a memset() is better.
No initialization at all is also a valid solution. I do not have any
warning in this case.
>> ../../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.
It looks like GCC do not like to have extern statement _inside_ a function
Moving the extern declaration outside a function fixes the warning.
Bye
--
Dr. Ludovic Rousseau
------------------------------------------------------------------------------
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
