Re: [PATCH] use xstrdup, not strdup in ll-merge.c

[Alex Riesen <raa.lkml@xxxxxxxxx>]

2009/6/15 Jim Meyering <jim@xxxxxxxxxxxx>:
> Alex Riesen wrote:
>> 2009/6/15 Jim Meyering <jim@xxxxxxxxxxxx>:
>>>
>>> Exactly.  This is why I think it's not worthwhile to invest in
>>> a more precise diagnostic, here.
>>
>> I disagree. It is already hard to find starting point for debugging if
>> the failed code is just a layer: the config of ll-merge is called not only
>> from the merge drivers, but also indirectly from the programs which
>> call the merge itself. Now, go figure where has it failed...
>
> If you're convinced of the value of such a change, go for it.

As much as I'd like to know as much as possible about why something
failed, I can't make a failure handling automatically simple (given the tools).
I do suggest using goto to handle this particular OOM (below), but it looks
almost too ugly. Maybe I am just paranoid, and am overdoing this particular
case.

> Though it sounds like you're saying you'd prefer a stack trace.

I haven't though of that, but yes, that would be perfect (except for
a small problem where it is impossible to do in a portable way,
and there is know way to get information about).

The "goto oom" patch:

diff --git a/ll-merge.c b/ll-merge.c
index 31d6f0a..4977f20 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -230,8 +230,11 @@ static int read_merge_config(const char *var,
const char *value, void *cb)
 	int namelen;

 	if (!strcmp(var, "merge.default")) {
-		if (value)
+		if (value) {
 			default_ll_merge = strdup(value);
+			if (!default_ll_merge)
+				goto oom;
+		}
 		return 0;
 	}

@@ -266,6 +269,8 @@ static int read_merge_config(const char *var,
const char *value, void *cb)
 		if (!value)
 			return error("%s: lacks value", var);
 		fn->description = strdup(value);
+		if (!fn->description)
+			goto oom;
 		return 0;
 	}

@@ -289,6 +294,8 @@ static int read_merge_config(const char *var,
const char *value, void *cb)
 		 * status.
 		 */
 		fn->cmdline = strdup(value);
+		if (!fn->cmdline)
+			goto oom;
 		return 0;
 	}

@@ -296,10 +303,15 @@ static int read_merge_config(const char *var,
const char *value, void *cb)
 		if (!value)
 			return error("%s: lacks value", var);
 		fn->recursive = strdup(value);
+		if (!fn->recursive)
+			goto oom;
 		return 0;
 	}

 	return 0;
+oom:
+	return error("line merge: %s%s%s: out of memory", var,
+		     value ? " = ": "", value ? value: "");
 }

 static void initialize_ll_merge(void)
@@ -307,7 +319,8 @@ static void initialize_ll_merge(void)
 	if (ll_user_merge_tail)
 		return;
 	ll_user_merge_tail = &ll_user_merge;
-	git_config(read_merge_config, NULL);
+	if (git_config(read_merge_config, NULL) < 0)
+		exit(1);
 }

 static const struct ll_merge_driver *find_ll_merge_driver(const char
*merge_attr)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html