xgetXXbyYY: Improve error handling

[ferivoz]

• Treat another out of memory condition and fail with exit(13) consistently within the file.
• Avoid duplicated code in error handling

[hallyn]

Hi,

what do you think about this instead? :

diff --git a/libmisc/xgetXXbyYY.c b/libmisc/xgetXXbyYY.c
index e1bf70f6..ced4fdc5 100644
--- a/libmisc/xgetXXbyYY.c
+++ b/libmisc/xgetXXbyYY.c
@@ -47,7 +47,7 @@
 /*@null@*/ /*@only@*/LOOKUP_TYPE *XFUNCTION_NAME (ARG_TYPE ARG_NAME)
 {
 #if HAVE_FUNCTION_R
-	LOOKUP_TYPE *result=NULL;
+	LOOKUP_TYPE *result=NULL, ret_result=NULL;
 	char *buffer=NULL;
 	/* we have to start with something */
 	size_t length = 0x100;
@@ -68,17 +68,12 @@
 		if ((0 == status) && (resbuf == result)) {
 			/* Build a result structure that can be freed by
 			 * the shadow *_free functions. */
-			LOOKUP_TYPE *ret_result = DUP_FUNCTION(result);
-			free(buffer);
-			free(result);
-			return ret_result;
+			ret_result = DUP_FUNCTION(result);
+			break;
 		}
 
-		if (ERANGE != status) {
-			free (buffer);
-			free (result);
-			return NULL;
-		}
+		if (ERANGE != status)
+			break;
 
 		if (length <= ((size_t)-1 / 4)) {
 			length *= 4;
@@ -91,7 +86,7 @@
 
 	free(buffer);
 	free(result);
-	return NULL;
+	return ret_result;
 
 #else /* !HAVE_FUNCTION_R */
 

[hallyn]

(The reason for my suggestion is that IMO it simplifies the success and failure exit cases)

[ferivoz]

In general I would agree but one commit is about unifying the error handling between reentrant and non-reentrant versions (the else block of the preprocessor if).

If we use your code, we should do the same in the else block, i.e. no exit if memory exhaustion occurred.

Do you want me to modify that code as well? Or do you prefer the exit version in this pull request?

[hallyn]

In general I would agree but one commit is about unifying the error handling between reentrant and non-reentrant versions (the else block of the preprocessor if).

If we use your code, we should do the same in the else block, i.e. no exit if memory exhaustion occurred.

Do you want me to modify that code as well? Or do you prefer the exit version in this pull request?

yeah, you're right, if we run out of memory we should exit, no sense going on after that.

[hallyn]

Jinkey, the codeql job is pretty fragile

[alejandro-colomar]

This looks like a typo in the NULL check. You probably meant ret_result == NULL, right?