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. > and it's just a hack that leaves the false impression of something > being fixed. I agree, requiring such things just by documenting them is fragile. > I think it would be better to add to the top of libusb.h the following: > > #if defined(__GNUC__) && ((__GNUC__ > 4) || ((__GNUC__ == 4) && > (__GNUC_MINOR__ >= 2))) > #pragma GCC diagnostic push > #pragma GCC diagnostic ignored "-Wcast-align" > #endif 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). > 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? Thank you for the review, And happy hacking :) -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercer...@gmail.com ------------------------------------------------------------------------------ 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
