We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 1388

Summary: Debian patch(es): ARM support and endian-ness issues
Product: SDL_net Reporter: manuel.montezelo
Component: miscAssignee: Sam Lantinga <slouken>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P2    
Version: unspecified   
Hardware: Other   
OS: Linux   
Attachments: arm.patch
endian.patch

Description manuel.montezelo 2012-01-20 12:26:38 UTC
Created attachment 790 [details]
arm.patch

Debian contains the following related patches since 2003.  In the patches descriptions it claims to have been forwarded upstream, but I don't know if they have been handled or rejected for some reason.

I am re-reporting them now in the case that it's useful, especially because ARM systems now are very much in the front line, so either one of those:

a) If it's a valid patch, it would be nice to have it upstream
b) If it's not, it's better to remove it from Debian

So here we go.

1) First Debian bug report: "bad aligned access considerations on ARM"

http://bugs.debian.org/212570

"ARM CPUs like my iPaq's fail on unaligned reads as well. See the following code for instance: [...] Moreover, AFAIK unaligned reads are always decomposed into two aligned reads on modern CPUs. I think it would be wise to drop them completely and always set SDL_DATA_ALIGNED to 1."

The accompanying patch "arm.patch" is trivial to understand and apply.


2) Second bug report: "SDLNet_(Read|Write)(16|32) assume host endianness is always LE"

http://bugs.debian.org/217221

"When SDL_DATA_ALIGNED is defined, SDLNet_Read16 etc. are defined as
byte-swapping macros. However, since network order is big endian, these
macros should not swap anything on big endian systems. Patch attached."

The accompanying patch "endian.patch" doesn't apply cleanly with 1.2.8 but it does with 1.2.7.  In any case it's also easy to understand: it removes conditional execution of code (SDL_BYTEORDER != SDL_BIG_ENDIAN) and keeps only the (SDL_BYTEORDER == SDL_BIG_ENDIAN) version.


Can you please comment if it's a valid fix or not?  If not I would like to remove the patches from the .deb packages as soon as possible.
Comment 1 manuel.montezelo 2012-01-20 12:27:08 UTC
Created attachment 791 [details]
endian.patch
Comment 2 Sam Lantinga 2012-01-20 16:16:46 UTC
These patches look good, thanks!
http://hg.libsdl.org/SDL_net/rev/97d99e5c44f4
http://hg.libsdl.org/SDL_net/rev/2eec18011e78