Re: [PATCH] use xstrdup, not strdup in ll-merge.c
- From
Alex Riesen <raa.lkml@gmail.com>
- Date
- Jun 15, 2009, 12:46 UTC
- Message-ID
- <81b0412b0906150546p34ff53ceo81ea59785cf9d18a@mail.gmail.com>
- In-Reply-To
- <877hzdlnt6.fsf@meyering.net>
2009/6/15 Jim Meyering <jim@meyering.net>:
Show 12 quoted lines
> Alex Riesen wrote: >> 2009/6/15 Jim Meyering <jim@meyering.net>: >>> >>> 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)