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 338

Summary: Wrong use of ctype functions
Product: SDL Reporter: Christian Biere <christianbiere>
Component: mainAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: 1.2.11   
Hardware: All   
OS: All   
Attachments: Adds "unsigned char" casts in macro definitions.

Description Christian Biere 2006-10-06 20:35:17 UTC 
SDL_string.c uses a couple of ctype routines but always passes a value of type "char" as argument. This is wrong on platforms with char being signed because the value might be negative. These functions accept only values representable by an unsigned char or EOF. Other values cause undefined behaviour which includes bogus return values and even crashes. Some platforms like Linux/glibc sweep such bugs under the carpet on many others out-of-range values cause indeed crashes. 
Therefore, char should always be casted to unsigned char for ctype functions like toupper, tolower, isidigit, isspace etc.
Comment 1 Christian Biere 2006-10-06 20:40:22 UTC 
Created attachment 166 [details]
Adds "unsigned char" casts in macro definitions.

The attached patch adds unsigned char casts for the macros SDL_isspace(), SDL_isdigit(), SDL_toupper(), SDL_tolower() when they are mapped to the ctype functions. This makes them slightly different from the ctype functions because EOF (usually -1) is not handled. However, SDL only uses them for strings not FILE I/O e.g. fgetc(), so this modification should be sufficient. The advantage is that it fixes all 3rd party code which might have been using these incorrectly as well (modulo the mentioned caveat).
Comment 2 Ryan C. Gordon 2007-02-12 06:15:03 UTC 
I would rather fix these at the individual calls in SDL_string.c ... changing the macro would be bad practice, since it adds an unexpected behaviour to it, even if it mostly works.

--ryan.
Comment 3 Ryan C. Gordon 2007-02-15 06:13:52 UTC 
I corrected SDL_string.c directly.

Fixed in svn revision #2980 for the 1.2 branch and #2981 for the 1.3 branch.

--ryan.