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 3992 - SDL_GetColorKey doesn't set error message
Summary: SDL_GetColorKey doesn't set error message
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.7
Hardware: x86_64 Linux
: P2 enhancement
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-05 15:13 UTC by Luke A. Guest
Modified: 2017-12-19 21:15 UTC (History)
2 users (show)

See Also:

Attachments
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 Luke A. Guest 2017-12-05 15:13:35 UTC 
SDL_GetColorKey does not set an error message on failure. The current source just returns -1.

The documentation https://wiki.libsdl.org/SDL_GetColorKey?highlight=%28%5CbCategoryAPI%5Cb%29%7C%28SDLFunctionTemplate%29 says to call SDL_GetError but that is useless in this case.
Comment 1 Sam Lantinga 2017-12-13 00:37:43 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/cf9c6a79123f
Comment 2 Sylvain 2017-12-13 09:25:19 UTC 
I don't think the second "SetError()" is a good idea:

"return SDL_SetError("Surface doesn't have a colorkey");"

Because the purpose and usage of SDL_GetColorKey() is also to tell whether the surface has a colorkey or not. And it will report false errors.
Comment 3 Sam Lantinga 2017-12-19 19:21:22 UTC 
I disagree, you need the error to distinguish between a NULL surface error and a surface that doesn't have a colorkey.
Comment 4 Ozkan Sezer 2017-12-19 20:03:46 UTC 
>  you need the error to distinguish between a NULL surface
>  error and a surface that doesn't have a colorkey.

The NULL surface error is already identifying itself by the
newly added SDL_InvalidParamError("surface"),  but why does
a surface w/o a colorkey need to set an error?
Comment 5 Sylvain 2017-12-19 20:42:16 UTC 
Yes, I mean't this and also: it simply reports a non-error as an error, which is confusing.

And it's bothering, if this non-error message replaces a real previous error message.
Comment 6 Sam Lantinga 2017-12-19 21:03:19 UTC 
If you previously called another API without a surface you'd think that you passed a NULL surface to this function, so an explicit state needs to be set if you're checking the error string for failure condition.

Really the API should be changed to return 1 if the colorkey is set, 0 if not, and -1 on error. We can make that change for SDL 2.1.
Comment 7 Sylvain 2017-12-19 21:15:07 UTC 
(In reply to Sam Lantinga from comment #6)
> If you previously called another API without a surface you'd think that you
> passed a NULL surface to this function, so an explicit state needs to be set
> if you're checking the error string for failure condition.

yes you're right, got it!