| Summary: | xf86-video-ati 6.8.0 uses #pragma pack incorrectly--server fails to load | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | xorg | Reporter: | Blair Sadewitz <bjs> | ||||||
| Component: | Driver/Radeon | Assignee: | xf86-video-ati maintainers <xorg-driver-ati> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> | ||||||
| Severity: | critical | ||||||||
| Priority: | medium | CC: | zerooa | ||||||
| Version: | unspecified | ||||||||
| Hardware: | x86 (IA32) | ||||||||
| OS: | NetBSD | ||||||||
| Whiteboard: | |||||||||
| i915 platform: | i915 features: | ||||||||
| Attachments: |
|
||||||||
|
Description
Blair Sadewitz
2008-02-20 21:24:21 UTC
This is probably a remnant from AMD where the atom parser came from. Please attach your patch and I'll take a look. Created attachment 14481 [details] [review] remove -DFGL_LINUX from src/Makefile.am The section of code directly influenced by this is located around line 57 in src/AtomBios/includes/CD_Structs.h. //#include "atombios.h" #if (PARSER_TYPE==DRIVER_TYPE_PARSER) #ifdef FGL_LINUX #pragma pack(push,1) #else #pragma pack(push) #pragma pack(1) #endif #endif I've never used #pragma pack myself, so if I am mistaken in my reasoning, please let me know. I can tell you, however, that not defining FGL_LINUX absolutely fixed the issue. If you need any further information, etc., please let me know. Regards, --Blair (In reply to comment #2) > Created an attachment (id=14481) [details] > remove -DFGL_LINUX from src/Makefile.am NACK - you expect developers on Linux to think of any potential implications of their changes on other OSs, but you didn't even bother looking up the change which added the flag with something like git-blame? Not defining FGL_LINUX causes build warnings on Linux. It looks like the code currently guarded by it needs to be diversified - as Alex pointed out, the ATOM BIOS code was previously only used on Windows and Linux. I did not mean that I wanted you to *remove this from the source*! I was merely reporting what I did to make it work on my system. Why would I want that to be removed? It surely has an application somewhere ... just not everywhere. (In reply to comment #5) > I did not mean that I wanted you to *remove this from the source*! Okay, sorry I misunderstood - I thought this was being submitted for inclusion. *** Bug 14770 has been marked as a duplicate of this bug. *** Created attachment 14789 [details] [review] autoconf patch Does this patch work for you? Yes, it does; thanks. --Blair committed: 74eb981287d76836327830bd51272f605a07e0cc Unfortunately this fix doesn't fix the issue for me (my bug was marked as a dupe of this one). While it LOOKS correct, with autoconf 2.5.9 FGL_LINUX is still defined after rerunning auto{conf,make} and building.
(In reply to comment #11) > Unfortunately this fix doesn't fix the issue for me (my bug was marked as a > dupe of this one). While it LOOKS correct, with autoconf 2.5.9 FGL_LINUX is > still defined after rerunning auto{conf,make} and building. > I guess $host_os isn't matching properly on your platform? Any chance you could find out what autoconf setup is needed in your case? I'm not really an autoconf expert. Modern NetBSD platforms usually have a tuple like: i386--netbsdelf (note the null second field) for the GNU platform, so something like *-netbsd*) is probably what should be used. Have you received any feedback from e.g. Solaris users about this issue? I'm wondering what other platforms might be affected ... Regards, --Blair (In reply to comment #13) > Have you received any feedback from e.g. Solaris users about this > issue? I'm wondering what other platforms might be affected ... I had a crash reported on solaris, which thinking back, may be the same issue. I'll follow up with that user as well. Aparrently $host_os expands to openbsd4.3 on this version (current, 4.3 beta) of my os. I'm stumped why this autoconf macro isn't triggering. I'll keep looking into it. Perhaps a better approach would be to figure out which operating systems _DO_ need FGL_LINUX to be defined and instead not define it by default? --Blair I talked to a Solaris developer and it's needed there as well and he had some other suggestions for how to do it. I'll try and get an improved patch committed soon. should be fixed in commit 766f464dfdfccadef23e4232f2bce5db22195513 Works perfectly for me. Thank you! If this is still an issue for anyone, please reopen. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.