Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 158646
Summary: | scorched3d not 64bit clean | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jeremy Katz <katzj> | ||||||
Component: | scorched3d | Assignee: | Hans de Goede <hdegoede> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | scop, toshio, wart | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | x86_64 | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2006-02-11 21:54:19 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 161694, 162161 | ||||||||
Attachments: |
|
Description
Jeremy Katz
2005-05-24 15:27:00 UTC
Created attachment 114792 [details]
Preliminary x86_64 fixes
Here's a preliminary patch. It gets scorched3d to build and run as far as the
menu and preferences. I don't know about getting further -- I don't have a
working 3d card right now and I'm getting errors about GLX that I assume mean I
need that. (If not, I'll open another ticket or research upstream.)
I think there are some problems with this patch but I need someone more
knowledgable about 64bit issues to confirm. When we have code like:
int foo(void *ptr) {
return (int)ptr;
}
I think it should be converted to:
long foo(void *ptr) {
return (long)ptr;
}
This is what's been done in this patch. I think it also has to address calling
code:
void main() {
int temp;
temp = foo();
}
and change to:
void main() {
long temp;
temp = foo();
}
Which hasn't been done yet.
Also, should pointers all be unsigned long? Or does long suffice?
Yes, pointers are long on 64bit arches. A different approach to take might be to do like glib does and define a macro like the following (and this is the anaconda version) /* 64 bit platforms, definitions courtesy of glib */ #if defined (__x86_64__) || defined(__ia64__) || defined(__alpha__) || defined(__powerpc64__) || defined(__sparc64__) || defined(__s390x__) #define POINTER_TO_INT(p) ((int) (long) (p)) #define INT_TO_POINTER(i) ((void *) (long) (i)) #else #define POINTER_TO_INT(p) ((int) (p)) #define INT_TO_POINTER(i) ((void *) (i)) #endif Then you just use that instead of the casting and it handles things properly without having to change any of the return types (similar macros can be done for the unsigned versions as well) Hmmm, even with these changes it still doesn't really work beyond the settings dialog. For now, I'm thinking an excludearch might make sense. The code looks like it's littered with int/long assumptions that you really can't make :( Created attachment 114850 [details]
my patch, for reference
Hmm.. Scorced CVS doesn't seem to have anything to help the situation either, best to make it excludearch for now as I don't expect to have this sorted out by FC4 release by any means. Feel free to commit that excudearch change, I still don't have CVS access :-/ Re comment #2. Why not just "#include <stdint.h>" and use "intptr_t" ? Because a number of upstream projects aren't ready to commit to C99. That may be (slowly) changing as I think the new Debian stable should have a new enough compiler, but it has traditionally been a problem as I've pushed 64bit fixes upstream. To the point that I stopped trying to go the c99 route I've updated the devel branch to 39.1 in CVS but not tagged yet, could someone check if it still needs patching for ExcludeArch before I request builds of it? Feel free to commit whatever is needed. (You'll need a new openal-devel from devel CVS to build 39.1; it's been built but not yet published.) I've taken ownership of scorched3d and will look into this (as time permits). I just managed to build this on FC-4 x86_64 using the devel sources. I only had to change BR: wxGTK-devel to wxGTK2-devel to get it to build on FC4. I was able to start it up and begin a game without any problems. Hi Wart, Thanks for looking, I know it compiles and runs kinda sorta in local play mode, but internet play is (was?) severly hoosed. It has taken me a lot of fiddling and looking at the code to get this fixed, but I'm about to commit a new 64bit patch to fe-cvs devel tree which should also fix internet play, both hosting a server and as a client. If you want I can give you the gory details, I need to type them out for upstream anyways. This is fixed in FE-CVS devel tree now, I'll psuh an update when I also have fixed the security issue reported in bug 161694. As promised here is an explanation of the fix as send upstream: The problem is NetServer.cpp and the destinationID's used elsewhere, destination ID's are unsigned int and thus are 32 bits even on a 64 bit arch. Since these also get exchanged over the network and I did't want to break (network) compatibility with the existing release I've choosen to keep the destinationID's unsigned int, which also minimizes changes outside of NetServer.cpp . The problem is that NetServer.cpp generates this ideas by casting TCPsocket to unsigned int, as TCPsocket is a ptr to a struct TCPsocket_, this means that NetServer is putting ptr's into ints this won't work on 64 bits since ints are only 32 bits and addresses are 64 bit, thus this will loose a part of the address. NetServer uses NetServerRead todo most of the work and when NetServer gets called it uses a std::map<TCPsocket, NetServerRead *> to find the NetServerRead to use. I've changed this to an std::map<unsigned int, NetServerRead *>. And I generate unique ideas in addClient. Thus removing the ugly and on 64 bit defect storing of TCPsocket addresses in unsigned int. For the rare case where the TCPsocket (getIpAddress) is actually needed by NetServer I've added a getSocket() to NetServerRead, so NetServer can get to the socket asociated with the destinationID. Since NetServerRead and NetServerXXXProtocol need access to both the socket and the destinationId and those are no longer the same I've extended NetServerRead to get passed both the socket and the destID on create and I've extended the relevant NetServerXXXProtocol methods to have both a socket and a destID as parameters. Besides that there are a few places where s3d stores integers in pointers, but except for needing a cast to compile on 64 bit this is no problem. scorched3d-39.1-2 has been built for devel / FC5 which fixes this. |