wClipboard: improve error handling

Unify error handling in ClipboardInitFormats() and actually handle the
return value of ClipboardInitSynthesizers(). Currently it always returns
TRUE, but this may change, so we'd better be clean.

Declare 'formatName' in wClipboardFormat as non-const. It is customary
in C to declare owned pointers as non-const because various deallocation
functions like free() take non-const pointers as arguments. Furthermore,
const char* is tightly associated with "string literals" which must not
be freed. Thus declaring this field as non-const is more accurate, and
removes that ugly void* cast from ClipboardInitFormats().

Unify error handling in ClipboardCreate(). The cleanup snippet should
not be repeated as it's prone to errors, like leaking the allocation of
clipboard->formats when ClipboardInitFormats() fails. Unified error
handling makes it much harder to forget resource cleanup on errors.

winpr/libwinpr/clipboard/clipboard.c

@@ -365,23 +365,22 @@ BOOL ClipboardInitFormats(wClipboard* clipboard)
 		ZeroMemory(format, sizeof(wClipboardFormat));
 		format->formatId = formatId;
 		format->formatName = _strdup(CF_STANDARD_STRINGS[formatId]);
-
 		if (!format->formatName)
-		{
-			int i;
-
-			for (i = formatId - 1; i >= 0; --i)
-			{
-				format = &(clipboard->formats[--clipboard->numFormats]);
-				free((void*)format->formatName);
-			}
-
-			return FALSE;
-		}
+			goto error;
 	}
 
-	ClipboardInitSynthesizers(clipboard);
+	if (!ClipboardInitSynthesizers(clipboard))
+		goto error;
+
 	return TRUE;
+
+error:
+	for (formatId = 0; formatId < clipboard->numFormats; formatId++)
+	{
+		free(clipboard->formats[formatId].formatName);
+		free(clipboard->formats[formatId].synthesizers);
+	}
+	return FALSE;
 }
 
 UINT32 ClipboardGetFormatId(wClipboard* clipboard, const char* name)
@@ -509,40 +508,37 @@ void ClipboardSetOwner(wClipboard* clipboard, UINT64 ownerId)
 wClipboard* ClipboardCreate()
 {
 	wClipboard* clipboard;
+
 	clipboard = (wClipboard*) calloc(1, sizeof(wClipboard));
+	if (!clipboard)
+		return NULL;
 
-	if (clipboard)
-	{
-		clipboard->nextFormatId = 0xC000;
-		clipboard->sequenceNumber = 0;
+	clipboard->nextFormatId = 0xC000;
+	clipboard->sequenceNumber = 0;
 
-		if (!InitializeCriticalSectionAndSpinCount(&(clipboard->lock), 4000))
-		{
-			free(clipboard);
-			return NULL;
-		}
+	if (!InitializeCriticalSectionAndSpinCount(&(clipboard->lock), 4000))
+		goto error_free_clipboard;
 
-		clipboard->numFormats = 0;
-		clipboard->maxFormats = 64;
-		clipboard->formats = (wClipboardFormat*) malloc(clipboard->maxFormats * sizeof(
-		                         wClipboardFormat));
+	clipboard->numFormats = 0;
+	clipboard->maxFormats = 64;
 
-		if (!clipboard->formats)
-		{
-			DeleteCriticalSection(&(clipboard->lock));
-			free(clipboard);
-			return NULL;
-		}
+	clipboard->formats = (wClipboardFormat*)
+		calloc(clipboard->maxFormats, sizeof(wClipboardFormat));
+	if (!clipboard->formats)
+		goto error_free_lock;
 
-		if (!ClipboardInitFormats(clipboard))
-		{
-			DeleteCriticalSection(&(clipboard->lock));
-			free(clipboard);
-			return NULL;
-		}
-	}
+	if (!ClipboardInitFormats(clipboard))
+		goto error_free_formats;
 
 	return clipboard;
+
+error_free_formats:
+	free(clipboard->formats);
+error_free_lock:
+	DeleteCriticalSection(&(clipboard->lock));
+error_free_clipboard:
+	free(clipboard);
+	return NULL;
 }
 
 void ClipboardDestroy(wClipboard* clipboard)

winpr/libwinpr/clipboard/clipboard.h

@@ -30,7 +30,7 @@ typedef struct _wClipboardSynthesizer wClipboardSynthesizer;
 struct _wClipboardFormat
 {
 	UINT32 formatId;
-	const char* formatName;
+	char* formatName;
 
 	UINT32 numSynthesizers;
 	wClipboardSynthesizer* synthesizers;