Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed many compiler warning about sign and size mismatch #509

Closed
wants to merge 1 commit into from

Conversation

seanm
Copy link
Contributor

@seanm seanm commented Dec 16, 2018

  • added various casts
  • assed some asserts where the casts made assumptions
  • enabled additional warnings in Xcode project

@LudovicRousseau
Copy link
Member

I would prefer to have:

  • different commit for different issues
  • the compiler warning the commit is fixing

@seanm
Copy link
Contributor Author

seanm commented Dec 17, 2018

I didn't really fix any 'issue', I just enabled some warning flags in the Xcode project (Xcode/common.xcconfig) and fixed them.

Not sure what kind of granularity you think'd be best... do you find it too big to review?

@LudovicRousseau
Copy link
Member

Without the compiler warnings I can't tell if the change is correct or needed.
My preferred form of patch is like this one LudovicRousseau@8e9c6f3
Yes, it is much more work to create the commit message.

Maybe another libusb maintainer will accept your patch as-is.

@hjelmn
Copy link
Member

hjelmn commented Dec 17, 2018

@LudovicRousseau That seems reasonable. I think he could put them all in one commit if the warnings are all in the commit message. Might get a little messy though. Maybe break it down by component (darwin, io, etc).

@hjelmn
Copy link
Member

hjelmn commented Dec 17, 2018

I want this all for the next release for sure though. There is no rush. I was going to see about cutting an rc next week sometime.

@seanm
Copy link
Contributor Author

seanm commented Dec 17, 2018

Well, the warning messages I can create for you just by checking out master and building, here you go:

from clang -Wshorten-64-to-32

/descriptor.c:65:23: Implicit conversion loses integer precision: 'int' to 'uint16_t' (aka 'unsigned short')
/descriptor.c:1166:19: Implicit conversion loses integer precision: 'int' to 'uint16_t' (aka 'unsigned short')
/os/darwin_usb.c:606:45: Implicit conversion loses integer precision: 'int' to 'uint8_t' (aka 'unsigned char')
/os/darwin_usb.c:711:28: Implicit conversion loses integer precision: 'int' to 'UInt16' (aka 'unsigned short')
/os/darwin_usb.c:713:23: Implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'UInt16' (aka 'unsigned short')
/os/darwin_usb.c:1011:42: Implicit conversion loses integer precision: 'UInt16' (aka 'unsigned short') to 'uint8_t' (aka 'unsigned char')
/os/darwin_usb.c:1174:66: Implicit conversion loses integer precision: 'int' to 'UInt8' (aka 'unsigned char')
/os/darwin_usb.c:1183:26: Implicit conversion loses integer precision: 'int' to 'UInt8' (aka 'unsigned char')
/os/darwin_usb.c:1249:84: Implicit conversion loses integer precision: 'int' to 'UInt8' (aka 'unsigned char')
/os/darwin_usb.c:1273:87: Implicit conversion loses integer precision: 'int' to 'uint8_t' (aka 'unsigned char')
/os/darwin_usb.c:1296:50: Implicit conversion loses integer precision: 'int' to 'uint8_t' (aka 'unsigned char')
/os/darwin_usb.c:1311:52: Implicit conversion loses integer precision: 'int' to 'uint8_t' (aka 'unsigned char')
/os/darwin_usb.c:1432:87: Implicit conversion loses integer precision: 'int' to 'UInt8' (aka 'unsigned char')
/os/darwin_usb.c:1501:70: Implicit conversion loses integer precision: 'int' to 'UInt8' (aka 'unsigned char')
/os/darwin_usb.c:1502:73: Implicit conversion loses integer precision: 'int' to 'UInt8' (aka 'unsigned char')
/os/darwin_usb.c:1528:50: Implicit conversion loses integer precision: 'int' to 'uint8_t' (aka 'unsigned char')
/os/darwin_usb.c:1693:72: Implicit conversion loses integer precision: 'unsigned int' to 'UInt16' (aka 'unsigned short')

from cppcheck

[examples/sam3u_benchmark.c:58]: (warning) %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'.
[examples/sam3u_benchmark.c:62]: (warning) %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'.
[libusb/os/haiku_usb_backend.cpp:246]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour

I can amend the commit message with that if you'd like.

- added various casts
- added some asserts where the casts made assumptions
- enabled additional warnings in Xcode project (especially -Wshorten-64-to-32)
@seanm
Copy link
Contributor Author

seanm commented Jan 17, 2019

@hjelmn @LudovicRousseau I've fixed the merge conflicts...

@hjelmn
Copy link
Member

hjelmn commented Jan 17, 2019

Thanks @seanm. I plan to do another batch this weekend with the hope of getting a release out before the end of the month.

@hjelmn hjelmn closed this in 8a05a3f Jan 31, 2019
@mcuee mcuee added core Related to common codes Examples Examples labels Jul 4, 2021
Seneral pushed a commit to Seneral/libusb that referenced this pull request Sep 21, 2021
- added various casts
- added some asserts where the casts made assumptions
- enabled additional warnings in Xcode project (especially -Wshorten-64-to-32)

Closes libusb#509

Signed-off-by: Nathan Hjelm <hjelmn@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to common codes Examples Examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants