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 4046

Summary: Suggestion + implementation: SDL_strtok which will became as a platform idependent safe strtok()
Product: SDL Reporter: Vitaly Novichkov <admin>
Component: videoAssignee: Ozkan Sezer <sezeroz>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2 CC: sezeroz
Version: HG 2.0   
Hardware: All   
OS: All   
Attachments: SDL_strtokr.c
SDL_strtokr.c (V2)
SDL_strtokr.patch

Description Vitaly Novichkov 2018-01-14 00:06:16 UTC
While I worked on merging recent tonn of changes in SDL_mixer with my fork, I have found a weird and ugly deal with strtok over multiple platforms. Why not to provide own safe SDL_strtok which will use strtok_r or own-custom implementation that will be safe.

I have some already-made implementation I using now in SDL Mixer X you can take to use with SDL2 to provide SDL2_strtok:


/*
 * public domain strtok_r() by Charlie Gordon
 *
 *   from comp.lang.c  9/14/2007
 *
 *      http://groups.google.com/group/comp.lang.c/msg/2ab1ecbb86646684
 *
 *     (Declaration that it's public domain):
 *      http://groups.google.com/group/comp.lang.c/msg/7c7b39328fefab9c
 */
char *Mix_strtok_safe(char *str, const char *delim, char **nextp)
{
    char *ret;
    if (str == NULL) {
        str = *nextp;
    }

    str += strspn(str, delim);
    if (*str == '\0') {
        return NULL;
    }
    ret = str;

    str += strcspn(str, delim);
    if (*str) {
        *str++ = '\0';
    }

    *nextp = str;
    return ret;
}
Comment 1 Ozkan Sezer 2019-11-18 08:15:14 UTC
This uses libc functions to implement strtok().
I suggest adapting a version based on PDCLib
(https://github.com/DevSolar/pdclib.git / CC0 -
public domain license.) See string/strtok[_s].c
and _PDCLIB/_PDCLIB_strtok.c in there.

I can give a shot at this if required.
Comment 2 Sam Lantinga 2019-11-19 07:05:15 UTC
Sure, that sounds good. Please only pull in one file for the strtok implementation.
Comment 3 Ozkan Sezer 2019-11-19 09:50:02 UTC
Created attachment 4060 [details]
SDL_strtokr.c

See attached.  Adapted from:
https://github.com/DevSolar/pdclib/blob/master/functions/_PDCLIB/_PDCLIB_strtok.c

.. with following modifications:
- size checks removed (not a thing in strtok_r),
- __restrict keyword removed (no support in SDL),
- small whitecpace / indentation changes  so it
  follows SDL_string.c style.

The extern declaration at the top to go into SDL_stdinc.h.

Please review and test carefully and extensively.

- Ok to add to hg?

- Is the license notification Ok?
Comment 4 Ozkan Sezer 2019-11-19 10:45:51 UTC
Created attachment 4061 [details]
SDL_strtokr.c (V2)

V2: Better format, fixed a preprocessor check for strtok_s,
better CC0 original license notification.
Comment 5 Vitaly Novichkov 2019-11-19 18:12:55 UTC
Looks fine, it should work!
Comment 6 Ozkan Sezer 2019-11-20 00:10:06 UTC
Created attachment 4062 [details]
SDL_strtokr.patch

OK then, here is a patch version:

Build tested:
- Linux (autotools & cmake)
- MinGW (cross- from Linux, autotools & cmake)
- VisualC (VS2017)

NOT build tested:
- Xcode-iOS
- Xcode
- VisualC-WinRT

Config headers NOT touched:
- SDL_config_pandora.h, SDL_config_psp.h
 (don't know whether they have strtok_r)

OK?  (Please review and run-test.)
Comment 7 Ozkan Sezer 2019-11-20 15:26:52 UTC
I merged the tree layout changes of Vitaly Novichkov
to help with his preparing his changesets.

Build status: Autotools, Xcode, and VicualC works for me.
Xcode-iOS and VisualC-WinRT projects are updated but NOT
tested at all: please test/update them as needed  (I can
not do that myself.)
Comment 8 Ozkan Sezer 2019-11-20 15:45:21 UTC
(In reply to Ozkan Sezer from comment #7)
Obviously posted that to wrong bug entry. (shame..)
Comment 9 Ozkan Sezer 2019-11-20 17:43:01 UTC
Pushed http://hg.libsdl.org/SDL/rev/d6decc5d2464

Closing.  If any issues show up, add a note here.