Re: [PATCH v3] usbutils: convert to libusb-1.0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tuesday, November 30, 2010 20:43:57 Greg KH wrote:
> On Thu, Nov 18, 2010 at 06:03:45PM -0500, Mike Frysinger wrote:
> > On Thursday, November 18, 2010 17:04:38 Greg KH wrote:
> > > On Thu, Nov 18, 2010 at 04:08:26PM -0500, Mike Frysinger wrote:
> > > > -#if (__BYTE_ORDER == __LITTLE_ENDIAN)
> > > > -  #define le16_to_cpu(x) (x)
> > > > -#elif (__BYTE_ORDER == __BIG_ENDIAN)
> > > > -  #define le16_to_cpu(x) bswap_16(x)
> > > > -#else
> > > > -  #error missing BYTE_ORDER
> > > > -#endif
> > > > +#define le16_to_cpu(x) libusb_cpu_to_le16(libusb_cpu_to_le16(x))
> > > 
> > > This line causes warnings when building.  And are you sure it's right?
> > > Is that how libusb wants to be used?  Why not just use the
> > > libusb_le16_to_cpu() macro instead?
> > 
> > it isnt causing warings for me ... what warnings are you seeing ?
> 
>   CC     lsusb-lsusb.o
> lsusb.c: In function âdump_configâ:
> lsusb.c:487:9: warning: declaration of â_tmpâ shadows a previous local
> lsusb.c:487:9: warning: shadowed declaration is here
> lsusb.c:487:9: warning: declaration of â_tmp2â shadows a previous local
> lsusb.c:487:9: warning: shadowed declaration is here
> lsusb.c: In function âdump_endpointâ:
> lsusb.c:745:18: warning: declaration of â_tmpâ shadows a previous local
> lsusb.c:745:18: warning: shadowed declaration is here
> lsusb.c:745:18: warning: declaration of â_tmp2â shadows a previous local
> lsusb.c:745:18: warning: shadowed declaration is here

i guess you're using -Wshadow ?  or your compiler enables that by default ?  
my understanding is that shadow warnings are not typically enabled by default 
due to the commonness of false positives.

> > it's doubled up because because it seems like the code wants to turn
> > "le16 to cpu" but libusb is doing "cpu to le16".  but looking at the
> > usage of this macro, perhaps i'm reading too much into the name.
> > 
> > i'll try and find a big endian machine with usable USB ...
> 
> What do you suggest we do?  Why can't we rely on the libusb version
> instead here?

when i eyeballed the code before, the two funcs did not appear to be doing the 
same thing.  if they are indeed, the same thing (but just poor naming), then 
there's no reason for this local macro to exist.  but i dont personally have 
any big endian systems anymore for me to verify the behavior with actual 
hardware.  my quad G5's cooling system decided to kill itself.

so i suggest someone step up who is running Linux on a PPC and verify with 
some actual USB devices that this macro is not necessary and the libusb one is 
sufficient.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux