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 2989 - Memory loss in clipboard_testClipboardTextFunctions
Summary: Memory loss in clipboard_testClipboardTextFunctions
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: *don't know* (show other bugs)
Version: HG 2.0
Hardware: All All
: P2 critical
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-21 06:26 UTC by Nitz
Modified: 2015-05-26 13:17 UTC (History)
1 user (show)

See Also:

Attachments
Patch for memory loss (766 bytes, patch)
2015-05-21 06:26 UTC, Nitz 		Details | Diff
New Patch is attached (854 bytes, patch)
2015-05-25 05:08 UTC, Nitz 		Details | Diff
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 Nitz 2015-05-21 06:26:55 UTC 
Created attachment 2158 [details]
Patch for memory loss

In File testautomation_clipboard.c

under funciton 
int
clipboard_testClipboardTextFunctions(void *arg)


charResult = SDL_GetClipboardText(); 

here charResult memory definitely getting lost at two places.

Patch is attached for solution


Regards,
Nitin
Comment 1 Philipp Wiesemann 2015-05-21 20:47:44 UTC 
The first SDL_free() added by the patch might crash if the clipboard is empty (because the variable charResult will contain its undefined default value then).
Comment 2 Nitz 2015-05-22 10:47:28 UTC 
No it will not crash in this case also because 
anyways it will create memory on heap by using SDL_strdup("");

So to free the memory on heap we have to do SDL_free() here
otherwise memory will get lost.

If you want more security then you can add  SDLTest_AssertCheck here.
But according to me no need of it :)
Comment 3 Philipp Wiesemann 2015-05-22 20:25:08 UTC 
If the first SDL_free() added by the patch would be inside the brackets above, it would crash (because there a value is set to charResult). Otherwise it is possible that it only contains undefined data.
Comment 4 Nitz 2015-05-25 05:08:16 UTC 
Created attachment 2160 [details]
New Patch is attached
Comment 5 Nitz 2015-05-25 05:13:51 UTC 
I attached new patch according to your concern.
If you refer clipboard_testGetClipboardText(void *arg) function in same file,
It is handling the charResult same as the patch attached.

int
clipboard_testGetClipboardText(void *arg)
{
    char *charResult;
    charResult = SDL_GetClipboardText();
    SDLTest_AssertPass("Call to SDL_GetClipboardText succeeded");

    SDL_free(charResult);

    return TEST_COMPLETED;
}

I have also read the description of SDL_GetClipboardText on SDL Wiki
and there it is written that:
Use SDL_GetClipboardText function to get UTF-8 text from the clipboard, which must be freed with SDL_free(). 

Thanks!!!
Comment 6 Sam Lantinga 2015-05-26 13:17:13 UTC 
Fixed, thanks!
https://hg.libsdl.org/SDL/rev/062bfde7330a