2013/1/29 Pete Batard <p...@akeo.ie>:
> Just applied 3&4 but dropped 1&2. 1&2 produce sign conversion warnings
> in MSVC, that seem to be justified since we're comparing _packet with
> transfer->num_iso_packets, that is defined as an int. I don't quite get
> why we would want to switch to unsigned there.

We have:
  1364  static inline unsigned char *libusb_get_iso_packet_buffer_simple(
  1365          struct libusb_transfer *transfer, unsigned int packet)
  1366  {
  1367          int _packet;
  1368  
  1369          /* oops..slight bug in the API. packet is an unsigned int, but 
we use
  1370           * signed integers almost everywhere else. range-check and 
convert to
  1371           * signed to avoid compiler warnings. FIXME for libusb-2. */
  1372          if (packet > INT_MAX)
  1373                  return NULL;
  1374          _packet = packet;
  1375  
  1376          if (_packet >= transfer->num_iso_packets)
  1377                  return NULL;
  1378  
  1379          return transfer->buffer + (transfer->iso_packet_desc[0].length
* _packet);
  1380  }

Line 1374 we copy packet (unsigned int) into _packet (int).
This line generates a warning. I tried to fix this warning.

Then at line 1376 we compare _packet (int) with
transfer->num_iso_packets (also int).
This line generates a warning (in MSVC) if _packet is changed to
unsigned int. I also have the compiler warning with my patch.
I agree my patch was just moving a warning from one line to another.
Not a good idea.

If I change the libusb_get_iso_packet_buffer_simple() API to use int
packet (instead of unsigned int) to solve the previous problem I then
have a new warning:
./libusb.h:1379:67: warning: implicit conversion changes signedness: 'int' to
      'unsigned int' [-Wsign-conversion]
  ...transfer->buffer + (transfer->iso_packet_desc[0].length * _packet);
                                                             ~ ^~~~~~~

Signedness issues are very hard to fix correctly without changing the
public API (and without using explicit casts)

> Also, could you please try to merge multiple one liners into a single
> patch? Even if I had to reject 2 of them, applying multiple one liners
> that deal with the same warning IN A ROW is a bit of a PITA, and
> requires more time than I'd like to spend on the task.

I prefer to fix only one problem in one patch.
It is then easier to find a bug using "git bisect" for example.

Thanks for merging the code.

Bye

-- 
 Dr. Ludovic Rousseau

------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d
_______________________________________________
libusbx-devel mailing list
libusbx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libusbx-devel

Reply via email to