Clean various warnings in low-level libraries.

trezor / trezord-go

:smiling_imp: Trezor Communication Daemon (written in Go)

GNU Lesser General Public License v3.0

241 stars 146 forks source link

Clean various warnings in low-level libraries. #268

Closed igor-hnizdo closed 1 year ago

igor-hnizdo commented 1 year ago

We are using old versions of libusb and hidapi that result in some warnings.

We could fix that by updating hidapi+libusb, but I am actually deadly afraid to do that (don't fix what works); I know what pain was to ensure that the USB layer really works.

So I just went through all the warnings, saw how the other libraries fixed them, and did the same.

The changes:

on mac, kIOMasterPortDefault is deprecated; we can just use 0 (or, as libusb does, add new constant and use that), in both libusb and hidapi
on windows, just use memcpy instead of useless strncpy in hidapi
on linux, use POSIX header instead of "incorrect" header that is redirected
and also remove unused libusb set_debug from go code

Note, this would all be solved if we used upstream versions of all the libraries, but again, I am super afraid to do that.

igor-hnizdo commented 1 year ago

note that this removes all the C warnings

igor-hnizdo commented 1 year ago

...to show that the memcpy thing is safe

https://onlinegdb.com/5J1kH6loa

...if the original string would not be null terminated, strlen would fail anyway. I think it's fine like this.

tsusanka commented 1 year ago

Note, this would all be solved if we used upstream versions of all the libraries, but again, I am super afraid to do that.

Me and @prusnak did update the libraries (e.g. in https://github.com/trezor/trezord-go/commit/ca7483fea65600ebfefb63592cbb785797f41951 and https://github.com/trezor/trezord-go/commit/74d901af6943d98571f7f50c7f5f58e6a84e7b23) some time back. We did introduce some bugs but AFAIK they all were by forgetting some patches such as https://github.com/trezor/trezord-go/commit/5438e38dd37f8865e2ad27840480c7f81c1edfff.

So basically what I am saying is that I believe we can update, we just need to be careful. I have a small preference doing that, and just review and test very carefully. Any opinion on this @prusnak?

prusnak commented 1 year ago

Yeah, let's update hidapi and libusb.

igor-hnizdo commented 1 year ago

OK.

I can try to update both libraries, and document somewhere our patch-sets.