On Tue, 2 Jul 2013 23:18:12 +0400, Paul Fertser said: >On Tue, Jul 02, 2013 at 02:43:54PM -0400, Sean McBride wrote: >> I don't think Paul's patch should be committed, at least not as-is. >> It adds strange double casts (with no comment) that future readers >> of the code will wonder about > >What else can a reader think of seeing a double cast via void* >(assuming he doesn't consider libusb maintainers silly)? Even if that >alignment deal is not evident, he'd reread the doxygen comment right >above and see the alignment requirement for the buffer specified.
In my experience, most C programmers do not understand strict aliasing, type punning, alignment requirements, etc., so they could think all sorts of things. :) >What if some other compiler would be used that's as strict regarding >the matters? You'd have to add some other way to silence them. While >with the void* trick it should be guaranteed to work on all of them >(malloc() returns void* too, and it guarantees the strictest possible >alignment). I guess my objection is basically that the "void* trick" doesn't actually fix anything and makes it harder to find and fix this code in the future. It's easier to notice a pragma warning suppression in the future, comment it out, then actually fix the code. Trying to find redundant void* casts is much harder. >> This makes it clear a temporary hack is in place until the API can be fixed. > >What about documenting this in some special "API bugs" file or >documentation section? We should definitely have a bug for "make libusb -Wcast-align safe"... Cheers, -- ____________________________________________________________ Sean McBride, B. Eng s...@rogue-research.com Rogue Research www.rogue-research.com Mac Software Developer Montréal, Québec, Canada ------------------------------------------------------------------------------ This SF.net email is sponsored by Windows: Build for Windows Store. http://p.sf.net/sfu/windows-dev2dev _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
