Huge API change

I’m cleaning up the API for the 1.0 release everyone.

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

Comments?
-Sam Lantinga (slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

It would be a nice option to have SDL automatically printf() the errors (if
applicable to the current platform) rather than checking the return value on
every SDL function and doing your own printf().

-Garrett, WPI student majoring in Computer Science.

“He who joyfully marches in rank and file has already earned
my contempt. He has been given a large brain by mistake, since
for him the spinal cord would suffice.” -Albert EinsteinOn Tue, 27 Jul 1999, you wrote:

I’m cleaning up the API for the 1.0 release everyone.

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

If you set the SDL_DEBUG environment variable to 1, it will do that.
I don’t think it’s something that you want it to do in all cases, since
sometimes it sets the error variable on non-fatal errors.

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software> On Tue, 27 Jul 1999, you wrote:

I’m cleaning up the API for the 1.0 release everyone.

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

It would be a nice option to have SDL automatically printf() the errors (if
applicable to the current platform) rather than checking the return value on
every SDL function and doing your own printf().


“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

I’m cleaning up the API for the 1.0 release everyone.

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

Comments?

Too much trouble, it also would break all the code using SDL if you change
all the function calls. Instead, when you create a thread, allocate a
pointer to an error structure (initialized to zero). Then add a single
SDL library call to set the error pointer for the current thread. On
error, if the pointer is zero, nothing happens. If it is non-null, the
structure gets filled in. Gives you all the benefits, none of the
drawbacks.

-Dave

Too much trouble, it also would break all the code using SDL if you change
all the function calls. Instead, when you create a thread, allocate a
pointer to an error structure (initialized to zero). Then add a single
SDL library call to set the error pointer for the current thread. On
error, if the pointer is zero, nothing happens. If it is non-null, the
structure gets filled in. Gives you all the benefits, none of the
drawbacks.

I tried that, unfortunately it has drawbacks of it’s own.
You can’t get and save the error - you’re stuck with errno type behavior.
There’s additional overhead with the thread creation and error reporting.

I’m CC’ing Cliff Matthews who had much stronger objections, so he can
reply to the mailing list as well.

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

Comments?
-Sam Lantinga (slouken at devolution.com)
If there is no better way to do it add macros which call the functions
with a NULL pointer so existing code doesn’t break.

Stuart

Comments?
-Sam Lantinga (slouken at devolution.com)
If there is no better way to do it add macros which call the functions
with a NULL pointer so existing code doesn’t break.

I will definitely do that, keeping the existing function names,
and adding new ones which all take error parameters.

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

I tried that, unfortunately it has drawbacks of it’s own.
You can’t get and save the error - you’re stuck with errno type behavior.
There’s additional overhead with the thread creation and error reporting.

What’s wrong with errno behaviour? And if a person really cares about
the error result he/she can copy the structure to some safe area after
each function call.

I don’t like the idea of the SDL library calls becoming macros (unless
they already are which spooks me). One of the things about people deciding
to use a library is they don’t want the rug being pulled out from under
them. There is an element of faith in choosing to go with SDL, and this
talk of changing all the function calls, obsoleting old code, or adding
a macro hack to avoid that…well it all just turns me off.

Especially since I don’t see myself using this elaborate error stuff
anyway. I mean, the SDL function calls I am using can’t generate some
unexpected error, can they? For example if I call SDL_UpdateRect, what
all can go wrong if my coords are within the window? Likewise SDL_Init
already gives an error code, doesn’t it?

-Dave

I didn’t see Sam’s original post, but I’ll assume he presented to you
what I’ve been discussing with him.

“Sam” == Sam Lantinga writes:

>> Too much trouble, it also would break all the code using SDL if
>> you change all the function calls.

All the code using SDL is currently broken if it in any way counts on
SDL’s error facilities being re-entrant. If the code doesn’t count on
that, then using non-re-entrant macro wrappers will not break
anything.

>> Instead, when you create a thread, allocate a pointer to an
>> error structure (initialized to zero). Then add a single SDL
>> library call to set the error pointer for the current
>> thread. On error, if the pointer is zero, nothing happens. If
>> it is non-null, the structure gets filled in. Gives you all the
>> benefits, none of the drawbacks.

You’re left with a global variable that prevents re-entrancy. This is
an architectural mistake that people make over and over again and down
the road it bites people. You can trivially implement the
non-re-entrant versions of routines by just having your own global (or
thread-specific, but otherwise global) structure and then calling
macros that automatically pass this value in. Entire overhead is an
extra argument pushed per call.

How do you (or Sam) implement re-entrancy when you decide you (or he)
want(s) it? Lock the global variable (via a mutex), save the old
value, put in a new value, unlock it, then on your way out lock it,
restore the new value and unlock it? That’s a lot more overhead, and
either SDL programmers will all have to implement such a scheme on
their own, or it will have to be part of SDL. If it’s part of SDL,
you’re largely back to the situation where you have two APIs, one that
is re-entrant safe and one that isn’t, only the default is for the
safe one to be slower than necessary.

Sam> I tried that, unfortunately it has drawbacks of it's own.
Sam> You can't get and save the error - you're stuck with errno
Sam> type behavior.  There's additional overhead with the thread
Sam> creation and error reporting.

Sam> I'm CC'ing Cliff Matthews who had much stronger objections,
Sam> so he can reply to the mailing list as well.

“Too much trouble” has always frequently been people’s (especially in
the UNIX community) attitude towards handling errors properly. With
some wrapper macros, people who want to play fast and loose still can.
I don’t see any reason why existing code needs to be changed (except
SDL’s internal implementation).

How many things of architecturally dubious merit have been done in the
past only to cause havoc later on? We can head this particular
problem off with a well known technique. That leaves one fewer
alligator in the swamp. Maybe it’s overkill, but I’d much rather have
overkill than spend time tracing down mystery bugs in hell. I spend
enough time down there already.

Sam> 	-Sam Lantinga (slouken at devolution.com)

Sam> Lead Programmer, Loki Entertainment Software -- "Any
Sam> sufficiently advanced bug is indistinguishable from a
Sam> feature" -- Rich Kulawiec

–Cliff
@Clifford_T_Matthews

Instead, when you create a thread, allocate a pointer to an
error structure (initialized to zero). Then add a single SDL
library call to set the error pointer for the current
thread. On error, if the pointer is zero, nothing happens. If
it is non-null, the structure gets filled in. Gives you all the
benefits, none of the drawbacks.

You’re left with a global variable that prevents re-entrancy. This is
an architectural mistake that people make over and over again and down
the road it bites people. You can trivially implement the
non-re-entrant versions of routines by just having your own global (or
thread-specific, but otherwise global) structure and then calling
macros that automatically pass this value in. Entire overhead is an
extra argument pushed per call.

I don’t see where the global variable comes in. I assume Sam’s
thead library maintains some structure per thread. Add an error pointer
to that structure, and initialize it to zero. It is meant to point
to an instance of an error structure, but on thread launch will
contain NULL. A thread will then allocate an error structure on
its own, and call the new SDL_Thread_Error function (or whatever it
is called) which updates that thread’s error structure pointer. On
error a thread will look for that pointer, and if it is non zero
the error structure it points to will be filled in. If the programmer
doesn’t call SDL_Thread_Error with a unique error structure, he
deserves the resultant bug(s).

How do you (or Sam) implement re-entrancy when you decide you (or he)
want(s) it? Lock the global variable (via a mutex), save the old
value, put in a new value, unlock it, then on your way out lock it,

I’m not an expert on thread programming, I usually do everything in
one single thread (ie the main code). In fact I have yet to actually
write threaded code, although I have used unix fork() once or twice.

The talk of lock/unlock/mutex…It leaves me thinking of race conditions,
lockups where both threads are waiting for the other to unlock something
(I won’t release mine until he releases his :^). In my mind all this
is theory anyway. In my bomber game where I replaced my sound fork()
code with SDL’s sound callback mechanism, I had the main program
pass sound commands in a circular buffer with a put pointer that
wraps around. The callback function, in another thread (sdl audio
is done with threads I assume), looks into the same buffer but with
a different pointer (“take”). When take!=put, the audio callback
function will keep fetching commands until the pointers are the
same again. There is no locking, no conflicts, and no problems.

I would imagine most thread stuff that requires locking could be
implemented in some other way so as not to require locking. But it is
just a feeling, not from actual experience. I’ve been programming for
23 years and my instincts have usually served me well :^).

All I really care about is that the SDL library function calls don’t
change. Now, if they must change because they weren’t set up correctly
with threads in mind, then I’d say make the minimum changes necessary to
fix the problem. But speaking of changing every SDL function call is
a scary prospect. I picture a nightmare of tech support emails, “My
program won’t compile with the new version but it works with the old,”
“I’m getting core dumps on programs that worked before I upgraded to a
new version of SDL,” etc, etc.

I’ve noticed things in the SDL-demos that don’t even match the latest
versions of the library…Speaking of Sam or whoever making sweeping
changes to the library makes me nervous. How many bugs are going to be
introduced? Will the examples be updated correctly? Lots of details
haven’t been dealt with before as regards minor changes…
I’m thinking specifically of the audio stereo stuff, where stereo
used to be a flag in the init audio function, but then switched to
a member in the structure (channels). And the audio example program
on the latest docs said to use the old method, not the new one!

-Dave

I don’t see where the global variable comes in. I assume Sam’s
thead library maintains some structure per thread. Add an error pointer
to that structure, and initialize it to zero. It is meant to point
to an instance of an error structure, but on thread launch will
contain NULL. A thread will then allocate an error structure on
its own, and call the new SDL_Thread_Error function (or whatever it
is called) which updates that thread’s error structure pointer. On
error a thread will look for that pointer, and if it is non zero
the error structure it points to will be filled in. If the programmer
doesn’t call SDL_Thread_Error with a unique error structure, he
deserves the resultant bug(s).

That’s the current implementation. Part of the trickiness is the fact
that per-thread variables aren’t easy to implement in a safe way.

The talk of lock/unlock/mutex…It leaves me thinking of race conditions,
lockups where both threads are waiting for the other to unlock something
(I won’t release mine until he releases his :^). In my mind all this
is theory anyway. In my bomber game where I replaced my sound fork()
code with SDL’s sound callback mechanism, I had the main program
pass sound commands in a circular buffer with a put pointer that
wraps around. The callback function, in another thread (sdl audio
is done with threads I assume), looks into the same buffer but with
a different pointer (“take”). When take!=put, the audio callback
function will keep fetching commands until the pointers are the
same again. There is no locking, no conflicts, and no problems.

I would imagine most thread stuff that requires locking could be
implemented in some other way so as not to require locking. But it is
just a feeling, not from actual experience. I’ve been programming for
23 years and my instincts have usually served me well :^).

What you have in bomberman is busy waiting. If you wanted to wait
on a mutex when the buffer was full, it would get a little more complex. :slight_smile:
Just an observation.

All I really care about is that the SDL library function calls don’t
change. Now, if they must change because they weren’t set up correctly
with threads in mind, then I’d say make the minimum changes necessary to
fix the problem. But speaking of changing every SDL function call is
a scary prospect. I picture a nightmare of tech support emails, “My
program won’t compile with the new version but it works with the old,”
“I’m getting core dumps on programs that worked before I upgraded to a
new version of SDL,” etc, etc.

I am ambivalent about the change. It would be the “Right Thing To Do”,
but the API gets more complicated, and the benefit only touches a few
people:

How many people actually care about the precise return values of functions?
Part of this is lack of planning on the design of the API, i.e.
I didn’t have a set way of returning error codes and a set of meanings
for those error codes. Adding an error pointer parameter adds an official
way of representing both the error code and the raw information needed to
display an error message in a thread-safe way. However, it adds complexity
to the API which isn’t necessarily desireable.

So. The question is, what are the benefits v.s. the pains?

Pro:
Thread-safe error handling
Internationalized error messages
Function-specific errors (i.e. no global errno value)
Better defined error conditions

Con:
Adds complexity to the API

So, what is the actual effect of the proposed change on the API?

Let’s take for example SDL_BlitSurface()

Old method:
if ( SDL_BlitSurface(src, &srcrect, dst, &dstrect) < 0 ) {
fprintf(stderr, “Blit failed: %s\n”, SDL_GetError());
}

New method:
if ( SDL_BlitSurface(&error, src, &srcrect, dst, &dstrect) < 0 ) {
fprintf(stderr, “Blit failed: %s\n”, SDL_GetError(&error));
/* This is actually a possible error */
if ( error.code == SDL_ERR_LOSTDIRECTXVIDEO ) {
ReloadSurfaces();
}
}
or if you don’t care about thread-safety (using a global error location):
if ( SDL_BlitSurface(NULL, src, &srcrect, dst, &dstrect) < 0 ) {
fprintf(stderr, “Blit failed: %s\n”, SDL_GetError(NULL));
}
or if you don’t care about errors at all:
SDL_BlitSurface(0, src, &srcrect, dst, &dstrect);

By the way, to put things in perspective, the new POSIX thread-safe way
of returning errors is to return -errno and have errno be an index into
a static table of error strings. I want SDL to be a little more flexible
in that you can have informative error messages like:
“Couldn’t set video mode: Must be less than 2048x2048”

On the pro side of the complexity, all SDL functions will take exactly
the same error parameter, so you won’t have to think about it once you
get used to it, and you’ll no longer need to check the return value of
functions, just check the error code at certain points in your code:

(Notice the changing of error to result)

SDL_ClearResult(&result);
screen = SDL_SetVideoMode(&result, 640, 480, 8, SDL_SWSURFACE);
if ( result.error ) {
	SDL_PrintError(&result, "Couldn't set video mode");
	exit(0);
}
loaded = SDL_LoadBMP(&result, "image.bmp");
image = SDL_DisplayFormat(&result, loaded);
SDL_FreeSurface(NULL, loaded);
if ( result.error ) {
	SDL_PrintError(&result, "Couldn't load and convert image.bmp");
	exit(0);
}
for ( i=0; i<screen->w/2; ++i ) {
	SDL_Rect rect;

	rect.x = i;
	rect.y = 0;
	rect.w = image->w;
	rect.h = image->h;
	SDL_BlitSurface(&result, image, NULL, screen, &rect);
	SDL_UpdateRects(&result, screen, 1, &rect);
}
if ( result.error ) {
	SDL_PrintError(&result, "Couldn't move image across screen");
	exit(0);
}

SDL_PrintError() could be defined as:
SDL_PrintError(SDL_result *result, const char *m)
{
char message[BUFSIZ];
fprintf(stderr, “%s: %s\n”, m, SDL_GetResult(&result, message, BUFSIZ));
}
It could also pop up a message box on Windows and/or MacOS.

I will decide whether or not to implement this based on user response,
since you all are the reason this library exists in the first place,
but either way this is decided, it must be done before 1.0.

I’ve noticed things in the SDL-demos that don’t even match the latest
versions of the library…Speaking of Sam or whoever making sweeping
changes to the library makes me nervous. How many bugs are going to be
introduced? Will the examples be updated correctly? Lots of details
haven’t been dealt with before as regards minor changes…

That’s because the development versions are places where new features
are tested, documentation isn’t updated, things may or may not work, etc.
The stable releases get all the bug fixes, and the API stays stable.

What happened with 0.9 is that it stayed out way too long and became
a stable version even though the documentation and examples hadn’t been
updated. This was because I didn’t want to go to version 1.0 without
addressing problems such as thread-safety, and internationalization
support which are important in production level code. However, they
were not so important that I took time off of Loki work to work on them.

I’m thinking specifically of the audio stereo stuff, where stereo
used to be a flag in the init audio function, but then switched to
a member in the structure (channels). And the audio example program
on the latest docs said to use the old method, not the new one!

I still need to update the docs. Once this issue is resolved, I update
the docs, and release the next stable version of SDL, hopefully 1.0. :slight_smile:

The stable version of SDL will stay, with no API changes, just bug-fixes,
as the version of SDL which can be used for production code. The next
development version will start having official MacOS support and I’ll
start on multi-monitor support.

BTW, I’m also planning a tutorial on making a game with SDL. :slight_smile:

So… what do you all think?
Cliff, you might want to join the SDL mailing list temporarily to
join the discussion. Lurkers, I want to hear from you too. :slight_smile:

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

Sounds like a pervasive and painful change for all involved.

Does SDL track any sort of data for its threads, such as platform-specific
wrappers? If so you could insert the error structure in there and use some
GetCurrentThread() and SetThreadError()-type calls to log errors to a thread.

Matt

/* Matt Slot, Bitwise Operator * One box, two box, yellow box, blue box. *

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

Sounds like a pervasive and painful change for all involved.

Does SDL track any sort of data for its threads, such as platform-specific
wrappers? If so you could insert the error structure in there and use some
GetCurrentThread() and SetThreadError()-type calls to log errors to a thread.

Actually, that’s the current thread-safe implementation.
I like it, but it has some drawbacks.

See my really long message for a full rundown. :slight_smile:

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

Sam Lantinga wrote:

So… what do you all think?

I say do it. It’s obviously The Right Thing (or at least A Right
Thing) from a technical standpoint, and as much as other people seem to
be complaining about the alleged pain this change will cause, it really
shouldn’t take more than a little find/replace work if you don’t care
about not taking full advantage of the new API.
s/SDL_BlitSurface(/SDL_BlitSurface(NULL, / isn’t that hard to do…

Nathan

Personally I’m for changing the API if it makes it more robust/useful
(Although passing an extra pointer into every function does make me cringe a
little - I’d rather have a SDL_GetLastError() interface :-).

Keep in mind that this is probably the last opportunity to fix any problems
in the API design - after 1.0 they have to be considered more or less set in
stone - you can add functions, but the crusty old ones will haunt you
forever…

My primary requirements are for meaningful error codes when something goes
wrong. I’ll be synthesizing my own (localised) error messages anyway and
throwing them in C++ exceptions.

Ben.–
Ben Campbell (Antipodean Straggler)
Programmer, CyberLife Technology Ltd
ben.campbell at cyberlife.co.uk

Sam Lantinga wrote:

I want to have thread-safe meaningful error reporting for each function.
That means passing in a pointer to an error structure, which can be checked
for an error later, to every function in the API.

Gawd, how many SDL programs are actually using thread support anyway?

Paul Lowe
spazz at ulink.net

“Sam” == Sam Lantinga writes:
Sam> [NOTE: I’m being blunt, so I’m sending this to you only. If
Sam> you’d like a less-blunt version sent to the list, I can do it
Sam> sometime after vacation.]

>> I don't see where the global variable comes in. I assume Sam's
>> thead library maintains some structure per thread. Add an error
>> pointer to that structure, and initialize it to zero. It is
>> meant to point to an instance of an error structure, but on
>> thread launch will contain NULL. A thread will then allocate an
>> error structure on its own, and call the new SDL_Thread_Error
>> function (or whatever it is called) which updates that thread's
>> error structure pointer. On error a thread will look for that
>> pointer, and if it is non zero the error structure it points to
>> will be filled in. If the programmer doesn't call
>> SDL_Thread_Error with a unique error structure, he deserves the
>> resultant bug(s).

Sam> That's the current implementation.  Part of the trickiness is
Sam> the fact that per-thread variables aren't easy to implement
Sam> in a safe way.

>> The talk of lock/unlock/mutex...It leaves me thinking of race
>> conditions, lockups where both threads are waiting for the
>> other to unlock something (I won't release mine until he
>> releases his :^). In my mind all this is theory anyway. In my
>> bomber game where I replaced my sound fork() code with SDL's
>> sound callback mechanism, I had the main program pass sound
>> commands in a circular buffer with a put pointer that wraps
>> around. The callback function, in another thread (sdl audio is
>> done with threads I assume), looks into the same buffer but
>> with a different pointer ("take"). When take!=put, the audio
>> callback function will keep fetching commands until the
>> pointers are the same again. There is no locking, no conflicts,
>> and no problems.

Most likely bomber is broken. It may be broken in such a way that the
error won’t show up on a non SMP machine, and here’s even a chance
that the compiler has been nice to him and set things up in such a way
that he won’t see the problem at all, but more likely there are little
glitches that are happening here and there that he isn’t aware of.

The callback function has to look at both take and put in order to
determine whether or not they are equal. Since they are operating
asynchronously without any synchronization, there may be times when
the value of put is incorrectly read from the callback function. Most
likely this just results in a slight glitch of the sound, so it’s not
a big deal. If the hardware he’s using does atomic writes of 4-byte
quantities, it may be that synchronization isn’t necessary, but since
his is presumably a C program and C doesn’t guarantee that the pointer
will be updated atomicly, his program is incorrect. A port or a new
piece of hardware may suddenly introduce bugs that he’s not aware of.

>> I would imagine most thread stuff that requires locking could
>> be implemented in some other way so as not to require
>> locking. But it is just a feeling, not from actual
>> experience. I've been programming for 23 years and my instincts
>> have usually served me well :^).

What hubris.

It’s highly unlikely that this guy has been programming SMP machines
for 23 years. I doubt he’s even been programming anything that is
marginally threaded for very long. He clearly doesn’t understand what
C specifies. He’s wearing his ignorance on his sleeve.

I too have been programming 23 years, including substantial time
programming OS-level things. I know how things should work, so I push
them hard and find tons of bugs both by running into them and visual
inspection of code. I have a good understanding of the difference
between what a language specifies and what an actual implementation
does. If the language (+ runtime) doesn’t specify it, you can’t count
on it happening just because you get lucky.

Sam> What you have in bomberman is busy waiting.  If you wanted to
Sam> wait on a mutex when the buffer was full, it would get a
Sam> little more complex. :) Just an observation.

Indeed.

>> All I really care about is that the SDL library function calls
>> don't change. Now, if they must change because they weren't set
>> up correctly with threads in mind, then I'd say make the
>> minimum changes necessary to fix the problem. But speaking of
>> changing every SDL function call is a scary prospect. I picture
>> a nightmare of tech support emails, "My program won't compile
>> with the new version but it works with the old," "I'm getting
>> core dumps on programs that worked before I upgraded to a new
>> version of SDL," etc, etc.

Sam> I am ambivalent about the change.  It would be the "Right
Sam> Thing To Do", but the API gets more complicated, and the
Sam> benefit only touches a few people:

Sam> How many people actually care about the precise return values
Sam> of functions?

Everyone does. They just don’t realize it. They care because there
are going to be people who will encounter real problems due to faulty
architectures and they won’t even know what’s going wrong. Let’s take
the mystery _vsnprintf bug as an example. NOBODY, (not even you or I)
has yet tracked that puppy down. Instead everyone is waiting for
someone else to do it, although the waiting is implicit for most of
the SDL users.

People may not realize their SDL-based Apps (at least ones using the
version of SDL that Executor ships with) won’t run on some NT
machines, so they may not think that they benefit from clean
architectures that help eliminate mystery bugs, but they do.

Sam> Part of this is lack of planning on the design of the API,
Sam> i.e.  I didn't have a set way of returning error codes and a
Sam> set of meanings for those error codes.  Adding an error
Sam> pointer parameter adds an official way of representing both
Sam> the error code and the raw information needed to display an
Sam> error message in a thread-safe way.  However, it adds
Sam> complexity to the API which isn't necessarily desireable.

I disagree. You’re confusing two separate issues. One is whether you
pass information back via a non-reentrant way, the other is how much
information you pass back. Even if you were just returning a number,
like errno, the correct way to do it is via passing in a pointer.
Just because there are tons of applications that don’t do this (and
are rarely multi-threaded) doesn’t mean it’s not an issue.

Sam> So.  The question is, what are the benefits v.s. the pains?

Sam> Pro: Thread-safe error handling Internationalized error
Sam> messages Function-specific errors (i.e. no global errno
Sam> value) Better defined error conditions

I think you really need to separate the issues.

Sam> Con: Adds complexity to the API

Adds optional complexity. You know that I’m a big fan of backward
compatibility. You should be able to make it so that the mod is
bakward compatible at the source level. (Something you didn’t do when
you changed the the ReadLE32 routines … grr…).

Sam> So, what is the actual effect of the proposed change on the
Sam> API?

Sam> Let's take for example SDL_BlitSurface()

Sam> Old method: if ( SDL_BlitSurface(src, &srcrect, dst,
Sam> &dstrect) < 0 ) { fprintf(stderr, "Blit failed: %s\n",
Sam> SDL_GetError()); }

Sam> New method: if ( SDL_BlitSurface(&error, src, &srcrect, dst,
Sam> &dstrect) < 0 ) { fprintf(stderr, "Blit failed: %s\n",
Sam> SDL_GetError(&error)); /* This is actually a possible error
Sam> */ if ( error.code == SDL_ERR_LOSTDIRECTXVIDEO ) {
Sam> ReloadSurfaces(); } } or if you don't care about
Sam> thread-safety (using a global error location): if (
Sam> SDL_BlitSurface(NULL, src, &srcrect, dst, &dstrect) < 0 ) {
Sam> fprintf(stderr, "Blit failed: %s\n", SDL_GetError(NULL)); }
Sam> or if you don't care about errors at all: SDL_BlitSurface(0,
Sam> src, &srcrect, dst, &dstrect);

DO NOT DO WHAT YOU PROPOSED.

IT IS EVIL TO CHANGE API’S OUT FROM UNDER PEOPLE.

You did this with SDL_ReadLE16 and friends. That was not the right
way to do it, and it broke code (mine).

Add “Err” to discriminate between the versions of the routines that take
an error pointer and the ones that don’t. Allow your existing users to
use the ones that don’t.

You’ll then have

   SDL_BlitSurface

and SDL_BlitSurfaceErr

No code of consequence will break.

Sam> By the way, to put things in perspective, the new POSIX
Sam> thread-safe way of returning errors is to return -errno and
Sam> have errno be an index into a static table of error strings.
Sam> I want SDL to be a little more flexible in that you can have
Sam> informative error messages like: "Couldn't set video mode:
Sam> Must be less than 2048x2048"

The POSIX way of doing things is broken, but at least POSIX
implementations have a little more constraint on their environment
than you do. Signals can’t interrupt system calls, a distinction that
SDL doesn’t have, so POSIX’s problems aren’t as bad as SDL’s, and
POSIX is trying to not make waves in a very big pond. I’m impressed
by the number of people using SDL (although not necessarily the
caliber), but there’s still room for a few more orders of magnitude of
growth. It really makes sense to do things right now while the user
base is still relatively small.

Sam> On the pro side of the complexity, _all_ SDL functions will
Sam> take exactly the same error parameter, so you won't have to
Sam> think about it once you get used to it, and you'll no longer
Sam> need to check the return value of functions, just check the
Sam> error code at certain points in your code:

Yup. People can also mimic the compatibility package and use their
own macros that jam that extra arg in there if it worries them so
much.

Sam> (Notice the changing of error to result)

Sam> 	SDL_ClearResult(&result); screen =
Sam> SDL_SetVideoMode(&result, 640, 480, 8, SDL_SWSURFACE); if (
Sam> result.error ) { SDL_PrintError(&result, "Couldn't set video
Sam> mode"); exit(0); } loaded = SDL_LoadBMP(&result,
Sam> "image.bmp"); image = SDL_DisplayFormat(&result, loaded);
Sam> SDL_FreeSurface(NULL, loaded); if ( result.error ) {
Sam> SDL_PrintError(&result, "Couldn't load and convert
Sam> image.bmp"); exit(0); } for ( i=0; i<screen->w/2; ++i ) {
Sam> SDL_Rect rect;

Sam> 		rect.x = i; rect.y = 0; rect.w = image->w;
Sam> rect.h = image->h; SDL_BlitSurface(&result, image, NULL,
Sam> screen, &rect); SDL_UpdateRects(&result, screen, 1, &rect); }
Sam> if ( result.error ) { SDL_PrintError(&result, "Couldn't move
Sam> image across screen"); exit(0); }

Sam> SDL_PrintError() could be defined as:
Sam> SDL_PrintError(SDL_result *result, const char *m) { char
Sam> message[BUFSIZ]; fprintf(stderr, "%s: %s\n", m,
Sam> SDL_GetResult(&result, message, BUFSIZ)); } It could also pop
Sam> up a message box on Windows and/or MacOS.

Sam> I will decide whether or not to implement this based on user
Sam> response, since you all are the reason this library exists in
Sam> the first place, but either way this is decided, it must be
Sam> done before 1.0.

There are three issues here:

  Do you use a global (or thread-global -- almost as bad) for passing
  back information?  There is only one right answer to this question if
  you want to have a uniform method of returning errors, and that is
  no, you don't use a global.

  Do you want to pass back more than a simple int as part of the error?
  I think a good case can be made for yes, but it's not nearly as clear
  cut as the first question.  This is a matter of convenience, the 
  previous question is a matter of correctness/flexibility (it is either
  incorrect and flexible or correct and inflexible; it can't be both).

  If you do want to pass back more information, do you want to do it
  via the API that you've constructed to date.  I think the proper answer
  is that it's too early to tell.  I simply haven't had enough time to 
  review what you're doing, and from the commentary you've passed on to
  me, I doubt that the other people who have looked at it are qualified
  to see things that will bite you in the future.  If you can't see
  why global (or thread-global) variables are bad, you're not qualified
  to judge.

>> I've noticed things in the SDL-demos that don't even match the
>> latest versions of the library...Speaking of Sam or whoever
>> making sweeping changes to the library makes me nervous. How
>> many bugs are going to be introduced? Will the examples be
>> updated correctly? Lots of details haven't been dealt with
>> before as regards minor changes...

Sam> That's because the development versions are places where new
Sam> features are tested, documentation isn't updated, things may
Sam> or may not work, etc.  The stable releases get all the bug
Sam> fixes, and the API stays stable.

Sam> What happened with 0.9 is that it stayed out way too long and
Sam> became a stable version even though the documentation and
Sam> examples hadn't been updated.  This was because I didn't want
Sam> to go to version 1.0 without addressing problems such as
Sam> thread-safety, and internationalization support which are
Sam> important in production level code.  However, they were not
Sam> so important that I took time off of Loki work to work on
Sam> them.

Exactly. I don’t blame you for what has happened with 0.9 at all.

>> I'm thinking specifically of the audio stereo stuff, where
>> stereo used to be a flag in the init audio function, but then
>> switched to a member in the structure (channels). And the audio
>> example program on the latest docs said to use the old method,
>> not the new one!

Sam> I still need to update the docs.  Once this issue is
Sam> resolved, I update the docs, and release the next stable
Sam> version of SDL, hopefully 1.0. :)

Personally I hope the next release is 0.10. I would like some time to
look at a lot of what you’ve done to see if there are other gotchas on
the way.

Sam> The stable version of SDL will stay, with no API changes,
Sam> just bug-fixes, as the version of SDL which can be used for
Sam> production code.  The next development version will start
Sam> having official MacOS support and I'll start on multi-monitor
Sam> support.

Sam> BTW, I'm also planning a tutorial on making a game with
Sam> SDL. :)


Sam> So... what do you all think?  Cliff, you might want to join
Sam> the SDL mailing list temporarily to join the discussion.
Sam> Lurkers, I want to hear from you too. :)

I’m about to go away on vacation. I’ll be gone the 29th through the
2nd and from the 6th through the 11th. I don’t think it makes sense
for me to join the discussion. Sorry. I like to think of myself as a
valuable resource. Perhaps we can cover more ground on the phone
sometime. I think you really botched the introduction of this by
suggesting that you would keep the same names and add a parameter.

Sam> 	-Sam Lantinga (slouken at devolution.com)

Sam> Lead Programmer, Loki Entertainment Software -- "Any
Sam> sufficiently advanced bug is indistinguishable from a
Sam> feature" -- Rich Kulawiec

–Cliff
@Clifford_T_Matthews

What hubris.

Yep.

I disagree. You’re confusing two separate issues. One is whether you
pass information back via a non-reentrant way, the other is how much
information you pass back. Even if you were just returning a number,
like errno, the correct way to do it is via passing in a pointer.
Just because there are tons of applications that don’t do this (and
are rarely multi-threaded) doesn’t mean it’s not an issue.

Don’t people say that in general it is bad to return information via
a passed in parameter?

Sam> Pro: Thread-safe error handling Internationalized error
Sam> messages Function-specific errors (i.e. no global errno
Sam> value) Better defined error conditions

I think you really need to separate the issues.

Okay. Internationalized errors and better defined error conditions
can be easily obtained by adding the result parameter. That was my
reasoning for grouping them together as pros.

Adds optional complexity. You know that I’m a big fan of backward
compatibility. You should be able to make it so that the mod is
bakward compatible at the source level. (Something you didn’t do when
you changed the the ReadLE32 routines … grr…).

Hmmm… that’s a really good point.

Add “Err” to discriminate between the versions of the routines that take
an error pointer and the ones that don’t. Allow your existing users to
use the ones that don’t.

You’ll then have

   SDL_BlitSurface

and SDL_BlitSurfaceErr

No code of consequence will break.

I’ll post this to the list.

  Do you use a global (or thread-global -- almost as bad) for passing
  back information?  There is only one right answer to this question if
  you want to have a uniform method of returning errors, and that is
  no, you don't use a global.

Right.

  Do you want to pass back more than a simple int as part of the error?
  I think a good case can be made for yes, but it's not nearly as clear
  cut as the first question.  This is a matter of convenience, the 
  previous question is a matter of correctness/flexibility (it is either
  incorrect and flexible or correct and inflexible; it can't be both).

Right. I lean towards passing back more than a simple int.
In many cases errors can occur which cannot be (or are not) covered by an
int error code.

  If you do want to pass back more information, do you want to do it
  via the API that you've constructed to date.  I think the proper answer
  is that it's too early to tell.  I simply haven't had enough time to 
  review what you're doing, and from the commentary you've passed on to
  me, I doubt that the other people who have looked at it are qualified
  to see things that will bite you in the future.

A few, but not most.

  If you can't see
  why global (or thread-global) variables are bad, you're not qualified
  to judge.

I can see it. It just doesn’t come up much when the only purpose to date
for using error codes is to print out messages. That will change in the
future, as the library gets used more.

Personally I hope the next release is 0.10. I would like some time to
look at a lot of what you’ve done to see if there are other gotchas on
the way.

I’m sure there are quite a few. :slight_smile:

I’m about to go away on vacation. I’ll be gone the 29th through the
2nd and from the 6th through the 11th. I don’t think it makes sense
for me to join the discussion. Sorry.

No problem.

I like to think of myself as a valuable resource.

As do I, which is why I always involve you in these kinds of questions.
I trust your judgement, even if I don’t always follow it. :slight_smile:

I think you really botched the introduction of this by
suggesting that you would keep the same names and add a parameter.

Yep. I just hate to have a separate set of API functions. sigh

I don’t think macros are the way to go, but I don’t want function call
overhead.

And… what was that clean way of doing the DLL loading?

Thanks Cliff. :slight_smile:

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

“Sam” == Sam Lantinga writes:

>> What hubris.
Sam> Yep.

>> I disagree.  You're confusing two separate issues.  One is
>> whether you pass information back via a non-reentrant way, the
>> other is how much information you pass back.  Even if you were
>> just returning a number, like errno, the *correct* way to do it
>> is via passing in a pointer.  Just because there are tons of
>> applications that don't do this (and are rarely multi-threaded)
>> doesn't mean it's not an issue.

Sam> Don't people say that in general it is bad to return
          ^

knowledgable
Sam> information via a passed in parameter?

No.

If you have a function, many people find it nicer to write it as

int func (int arg1, int arg2)

instead of

void func (int *retvalp, int arg1, int arg2)

but that’s not what we’re talking about.

Sam> Pro: Thread-safe error handling Internationalized error
Sam> messages Function-specific errors (i.e. no global errno
Sam> value) Better defined error conditions

>> I think you really need to separate the issues.

Sam> Okay.  Internationalized errors and better defined error
Sam> conditions can be easily obtained by adding the result
Sam> parameter.  That was my reasoning for grouping them together
Sam> as pros.

>> Adds *optional* complexity.  You know that I'm a big fan of
>> backward compatibility.  You should be able to make it so that
>> the mod is bakward compatible at the source level.  (Something
>> you didn't do when you changed the the ReadLE32 routines
>> ... grr...).

Sam> Hmmm.... that's a really good point.

>> Add "Err" to discriminate between the versions of the routines
>> that take an error pointer and the ones that don't.  Allow your
>> existing users to use the ones that don't.

>> You'll then have

>> SDL_BlitSurface and SDL_BlitSurfaceErr

>> No code of consequence will break.

Sam> I'll post this to the list.

>> Do you use a global (or thread-global -- almost as bad) for
>> passing back information?  There is only one right answer to
>> this question if you want to have a uniform method of returning
>> errors, and that is no, you don't use a global.

Sam> Right.

>> Do you want to pass back more than a simple int as part of the
>> error?  I think a good case can be made for yes, but it's not
>> nearly as clear cut as the first question.  This is a matter of
>> convenience, the previous question is a matter of
>> correctness/flexibility (it is either incorrect and flexible or
>> correct and inflexible; it can't be both).

Sam> Right.  I lean towards passing back more than a simple int.
Sam> In many cases errors can occur which cannot be (or are not)
Sam> covered by an int error code.

>> If you do want to pass back more information, do you want to do
>> it via the API that you've constructed to date.  I think the
>> proper answer is that it's too early to tell.  I simply haven't
>> had enough time to review what you're doing, and from the
>> commentary you've passed on to me, I doubt that the other
>> people who have looked at it are qualified to see things that
>> will bite you in the future.

Sam> A few, but not most.

>> If you can't see why global (or thread-global) variables are
>> bad, you're not qualified to judge.

Sam> I can see it.  It just doesn't come up much when the only
Sam> purpose to date for using error codes is to print out
Sam> messages.  That will change in the future, as the library
Sam> gets used more.

>> Personally I hope the next release is 0.10.  I would like some
>> time to look at a lot of what you've done to see if there are
>> other gotchas on the way.

Sam> I'm sure there are quite a few. :)

I honestly don’t now whether there are or not. I’ve had such blinders
on for a long time it’s almost hard to believe. OTOH, ARDI is doing
well, so I think I’ve been spending my time wisely.

>> I'm about to go away on vacation.  I'll be gone the 29th
>> through the 2nd and from the 6th through the 11th.  I don't
>> think it makes sense for me to join the discussion.  Sorry.

Sam> No problem.

>> I like to think of myself as a valuable resource.

Sam> As do I, which is why I always involve you in these kinds of
Sam> questions.  I trust your judgement, even if I don't always
Sam> follow it. :)

Well, there are three possibilities:

follow advice when given

never follow advice

follow advice after getting bitten repeatedly on the ass by the very same
things I indicted

>> I think you really botched the introduction of this by
>> suggesting that you would keep the same names and add a
>> parameter.

Sam> Yep.  I just hate to have a separate set of API functions.
Sam> *sigh*

Think of it as an optional parameter in a language that doesn’t
(properly) support them.

Sam> I don't think macros are the way to go, but I don't want
Sam> function call overhead.

Perhaps we should discuss this on the phone at some point.

Sam> And.. what was that clean way of doing the DLL loading?

I don’t know. I don’t have time to look into it, either.

Sam> Thanks Cliff. :)

Sam> 	-Sam Lantinga (slouken at devolution.com)

Sam> Lead Programmer, Loki Entertainment Software -- "Any
Sam> sufficiently advanced bug is indistinguishable from a
Sam> feature" -- Rich Kulawiec

–Cliff
@Clifford_T_Matthews

Please forgive my rant.

I am ambivalent about the change. It would be the “Right Thing To Do”,
but the API gets more complicated, and the benefit only touches a few
people:

I’m with you Sam. The right thing to do is in fact the right thing to do.
Let’s put this in perspective. One additional parameter per function call
is about as close to zero in performance hit as you can get, so I don’t
want to hear anything about how inefficient this would be. But also, the
errno method was a kludge 30 years ago, and it is today. It’s just the
Wrong Way to do it. Now, people who have learned to deal with the
idiosyncrasies of passing data through global variables have just patched
a sinking ship. It just shouldn’t be done. I can’t count the number of
times that I’ve been screwed by this kind of thinking. But when I learned
to do things the right way, and not rely on “Well, I’ll just have to be
careful, and not do N.” Six months later, I’ll do N and not realize why
the whole system came crashing down.

Pro:
Thread-safe error handling
Internationalized error messages
Function-specific errors (i.e. no global errno value)
Better defined error conditions

Right on. A well-designed threaded machine kicks the pants of anything in
a single thread. Look at BeOS… then look at MacOS. :wink: Anything that
makes threaded programming easier and more stable is a Good Thing.

Con:
Adds complexity to the API

Minimal.

Lets make the change now, while we are in v0.9, lest we are stuck with
something yucky for a long long time.

Again, please forgive my rant. I just wanted Sam to know there’s someone
on his side, and lend support.

-ChuckOn Wed, 28 Jul 1999, Sam Lantinga wrote: