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 - Wrong use of ctype functions
Summary: Wrong use of ctype functions
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: main (show other bugs)
Version: 1.2.11
Hardware: All All
: P2 normal
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-06 20:35 UTC by Christian Biere
Modified: 2007-02-15 06:13 UTC (History)
0 users

See Also:

Attachments
Adds "unsigned char" casts in macro definitions. (726 bytes, text/plain)
2006-10-06 20:40 UTC, Christian Biere 		Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.