Follow-up Comment #14, bug #17573 (project wesnoth): thomsew, I know that I said that I would have time to work on this on Friday, but soon afterward, I was informed about a friend's death and that his funeral was on Friday, during the time I had planned to spend on this. Regrettably, I had to change my plans following that.
With that said, I have a few suggestions to make. The first is that people should file bug reports with their distributions so that their distribution's package maintainers are notified of the upstream issue in libsdl. I filed a bug report for Gentoo Linux, which resulted in the package being patched within 48 hours: http://bugs.gentoo.org/show_bug.cgi?id=357687 Other distributions will likely take longer to patch this because they operate on a static release schedule, but it would probably be better to notify them sooner rather than later. I do not expect anyone to file bug reports for distributions other than the ones that they use, but as long as people volunteer to file bug reports for their own distributions, the upstream fix should trickle downstream faster than it would otherwise. Second, given the highly sophisticated (i.e. unpredictable) behavior that your work has shown memcpy() to exhibit on systems using glibc, it might be best to go by glibc's version number. That would mean that any system with a newer version of glibc and a patched older version of SDL would be forced to do unnecessary work, but I think that is the best that we can do without risking false positives or doing something more hackish like my original suggestion to implement a wrapper using Linus Torvalds' suggestion. Third, I noticed that your patches are using __GNUC__. Unfortunately, GCC's documentation indicates that the __GNUC__ preprocessor directive refers to the GCC compiler, rather than glibc: http://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html Since the use of GCC does not imply the use of glibc (especially on *BSD systems), applying a workaround because __GNUC__ is present is not the best thing to do. There appear to be preprocessor directives in features.h that we can use to detect the presence of glibc; specifically, __GLIBC__ and __GLIBC_MINOR__. features.h is glibc specific, so including it directly has portability issues. Fortunately, features.h appears to be included by glibc's C standard library header files, such that we can include features.h when it exists by including a C standard library header file. I have verified that both limits.h and stdio.h include features.h, so we could include either of them to get features.h's preprocessor directives when glibc is the system libc in portable manner. Doing detection of glibc version at compile time (e.g. #if __GLIBC__ >= 2 && __GLIBC_MINOR__ >= 13) would be problematic if Wesnoth is built on a distribution like RHEL where changes introduced in glibc 2.13 have been backported to some older version of glibc or if glibc is upgraded after Wesnoth has been compiled, which is often done on rolling distributions. We could use a version check at runtime to make the lives of rolling distribution maintainers easier while ignoring fork maintainers like RedHat. Fork maintainers are often paid to deal with these issues, so I think that letting them deal with the consequences should be okay. With that said, we could implement whatever workaround we choose to use on the systems likely to need it by doing the something like the following: #include <stdio.h> #if defined(__GLIBC__) && (SDL_VERSIONNUM(SDL_MAJOR_VERSION, SDL_MINOR_VERSION, SDL_PATCHLEVEL) < SDL_VERSIONNUM(1,2,15) #include <gnu/libc-version.h> int glibc, glibc_minor; sscanf(gnu_get_libc_version(), "%d.%d", &glibc, &glibc_minor); if (glibc >= 2 && glibc_minor >= 13) // Workaround code goes here else // Normal code goes here #else // Normal code goes here #endif". It might be a good idea to push the SDL library version detection into a runtime check so that a SDL upgrade would disable the workaround code on rolling distributions. Fridays are usually the days when I have spare time to work on these things, so I do not anticipate having time to write a proper patch until Friday (barring the possibility of another funeral). Anyone that is interested in doing this sooner is more than welcome to try; here is a link to the documentation for that: http://sdl.beuc.net/sdl.wiki/SDL_Linked_Version Also, I know that the above example code does not implement my own advice to do the test once, store the result and then check the stored result each time we need to blit, but I wanted to demonstrate the general idea. When I have time to do a proper patch, it will store the result as per my own advice. _______________________________________________________ Reply to this item at: <http://gna.org/bugs/?17573> _______________________________________________ Message sent via/by Gna! http://gna.org/ _______________________________________________ Wesnoth-bugs mailing list [email protected] https://mail.gna.org/listinfo/wesnoth-bugs
