From d64ea0f83bd7e676778f833c57f969a94518a28d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Jan 2015 20:57:37 -0500 Subject: [PATCH 1/5] git-compat-util: add xstrdup_or_null helper It's a common idiom to duplicate a string if it is non-NULL, or pass a literal NULL through. This is already a one-liner in C, but you do have to repeat the name of the string twice. So if there's a function call, you must write: const char *x = some_fun(...); return x ? xstrdup(x) : NULL; instead of (with this patch) just: return xstrdup_or_null(some_fun(...)); Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 44890d5b18f1f3..98cb78edf7b982 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -629,6 +629,11 @@ extern char *xgetcwd(void); #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x))) +static inline char *xstrdup_or_null(const char *str) +{ + return str ? xstrdup(str) : NULL; +} + static inline size_t xsize_t(off_t len) { if (len > (size_t) len) From 444069078687fc00586824a21eff9758fc4d0625 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Jan 2015 20:58:15 -0500 Subject: [PATCH 2/5] builtin/apply.c: use xstrdup_or_null instead of null_strdup This file had its own identical helper that predates xstrdup_or_null. Let's use the global one to avoid repetition. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/apply.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 8714a887203acd..4bb31fd6ae1fce 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -656,11 +656,6 @@ static size_t diff_timestamp_len(const char *line, size_t len) return line + len - end; } -static char *null_strdup(const char *s) -{ - return s ? xstrdup(s) : NULL; -} - static char *find_name_common(const char *line, const char *def, int p_value, const char *end, int terminate) { @@ -683,10 +678,10 @@ static char *find_name_common(const char *line, const char *def, start = line; } if (!start) - return squash_slash(null_strdup(def)); + return squash_slash(xstrdup_or_null(def)); len = line - start; if (!len) - return squash_slash(null_strdup(def)); + return squash_slash(xstrdup_or_null(def)); /* * Generally we prefer the shorter name, especially @@ -908,7 +903,7 @@ static void parse_traditional_patch(const char *first, const char *second, struc patch->old_name = name; } else { patch->old_name = name; - patch->new_name = null_strdup(name); + patch->new_name = xstrdup_or_null(name); } } if (!name) @@ -997,7 +992,7 @@ static int gitdiff_delete(const char *line, struct patch *patch) { patch->is_delete = 1; free(patch->old_name); - patch->old_name = null_strdup(patch->def_name); + patch->old_name = xstrdup_or_null(patch->def_name); return gitdiff_oldmode(line, patch); } @@ -1005,7 +1000,7 @@ static int gitdiff_newfile(const char *line, struct patch *patch) { patch->is_new = 1; free(patch->new_name); - patch->new_name = null_strdup(patch->def_name); + patch->new_name = xstrdup_or_null(patch->def_name); return gitdiff_newmode(line, patch); } From eaa541eb59aefa2c5e9e160c36a259d372c25711 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Jan 2015 20:58:33 -0500 Subject: [PATCH 3/5] builtin/commit.c: use xstrdup_or_null instead of envdup The only reason for envdup to be its own function is that we have to save the result in a temporary string. With xstrdup_or_null, we can feed the result of getenv() directly. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/commit.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index b0fe7847d3cbeb..b1cf2842408043 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -566,20 +566,14 @@ static void set_ident_var(char **buf, char *val) *buf = val; } -static char *envdup(const char *var) -{ - const char *val = getenv(var); - return val ? xstrdup(val) : NULL; -} - static void determine_author_info(struct strbuf *author_ident) { char *name, *email, *date; struct ident_split author; - name = envdup("GIT_AUTHOR_NAME"); - email = envdup("GIT_AUTHOR_EMAIL"); - date = envdup("GIT_AUTHOR_DATE"); + name = xstrdup_or_null(getenv("GIT_AUTHOR_NAME")); + email = xstrdup_or_null(getenv("GIT_AUTHOR_EMAIL")); + date = xstrdup_or_null(getenv("GIT_AUTHOR_DATE")); if (author_message) { struct ident_split ident; From 8c53f0719b04e0b6328c2e175e3c5d2dc8a0c282 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Jan 2015 20:59:09 -0500 Subject: [PATCH 4/5] use xstrdup_or_null to replace ternary conditionals This replaces "x ? xstrdup(x) : NULL" with xstrdup_or_null(x). The change is fairly mechanical, with the exception of resolve_refdup, which can eliminate a temporary variable. There are still a few hits grepping for "?.*xstrdup", but these are of slightly different forms and cannot be converted (e.g., "x ? xstrdup(x->foo) : NULL"). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 2 +- grep.c | 4 ++-- notes.c | 2 +- refs.c | 3 +-- remote.c | 4 ++-- shallow.c | 2 +- walker.c | 2 +- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/config.c b/config.c index 039647d247e0ac..400d2e47de5bda 100644 --- a/config.c +++ b/config.c @@ -1329,7 +1329,7 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha string_list_init(&e->value_list, 1); hashmap_add(&cs->config_hash, e); } - si = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL); + si = string_list_append_nodup(&e->value_list, xstrdup_or_null(value)); ALLOC_GROW(cs->list.items, cs->list.nr + 1, cs->list.alloc); l_item = &cs->list.items[cs->list.nr++]; diff --git a/grep.c b/grep.c index 99217dc04f5d04..f48a648a0d673e 100644 --- a/grep.c +++ b/grep.c @@ -1646,8 +1646,8 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, const void *identifier) { gs->type = type; - gs->name = name ? xstrdup(name) : NULL; - gs->path = path ? xstrdup(path) : NULL; + gs->name = xstrdup_or_null(name); + gs->path = xstrdup_or_null(path); gs->buf = NULL; gs->size = 0; gs->driver = NULL; diff --git a/notes.c b/notes.c index 5fe691dbcdfb9e..ee5f0e71f38b31 100644 --- a/notes.c +++ b/notes.c @@ -1006,7 +1006,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref, t->root = (struct int_node *) xcalloc(1, sizeof(struct int_node)); t->first_non_note = NULL; t->prev_non_note = NULL; - t->ref = notes_ref ? xstrdup(notes_ref) : NULL; + t->ref = xstrdup_or_null(notes_ref); t->combine_notes = combine_notes; t->initialized = 1; t->dirty = 0; diff --git a/refs.c b/refs.c index ffd45e92922ec5..32dce4e41c377c 100644 --- a/refs.c +++ b/refs.c @@ -1529,8 +1529,7 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag) { - const char *ret = resolve_ref_unsafe(ref, sha1, reading, flag); - return ret ? xstrdup(ret) : NULL; + return xstrdup_or_null(resolve_ref_unsafe(ref, sha1, reading, flag)); } /* The argument to filter_refs */ diff --git a/remote.c b/remote.c index ce785f8953bd8a..179bceff8a2ce2 100644 --- a/remote.c +++ b/remote.c @@ -975,8 +975,8 @@ struct ref *copy_ref(const struct ref *ref) cpy = xmalloc(sizeof(struct ref) + len + 1); memcpy(cpy, ref, sizeof(struct ref) + len + 1); cpy->next = NULL; - cpy->symref = ref->symref ? xstrdup(ref->symref) : NULL; - cpy->remote_status = ref->remote_status ? xstrdup(ref->remote_status) : NULL; + cpy->symref = xstrdup_or_null(ref->symref); + cpy->remote_status = xstrdup_or_null(ref->remote_status); cpy->peer_ref = copy_ref(ref->peer_ref); return cpy; } diff --git a/shallow.c b/shallow.c index 57f4afa6b40585..ee145743346f2c 100644 --- a/shallow.c +++ b/shallow.c @@ -21,7 +21,7 @@ void set_alternate_shallow_file(const char *path, int override) if (alternate_shallow_file && !override) return; free(alternate_shallow_file); - alternate_shallow_file = path ? xstrdup(path) : NULL; + alternate_shallow_file = xstrdup_or_null(path); } int register_shallow(const unsigned char *sha1) diff --git a/walker.c b/walker.c index 18a67d33cb55b7..adabcc9dfe7c96 100644 --- a/walker.c +++ b/walker.c @@ -232,7 +232,7 @@ int walker_targets_stdin(char ***target, const char ***write_ref) REALLOC_ARRAY(*write_ref, targets_alloc); } (*target)[targets] = xstrdup(tg_one); - (*write_ref)[targets] = rf_one ? xstrdup(rf_one) : NULL; + (*write_ref)[targets] = xstrdup_or_null(rf_one); targets++; } strbuf_release(&buf); From a46442f1675722eb68238b329a4a285f03f41dda Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Mon, 12 Jan 2015 20:59:26 -0500 Subject: [PATCH 5/5] blame.c: fix garbled error message The helper functions prepare_final() and prepare_initial() return a pointer to a string that is a member of an object in the revs->pending array. This array is later rebuilt when running prepare_revision_walk() which potentially transforms the pointer target into a bogus string. Fix this by maintaining a copy of the original string. Signed-off-by: Lukas Fleischer Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/blame.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 3838be2b0274fb..699109b4a9d766 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2390,7 +2390,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, return commit; } -static const char *prepare_final(struct scoreboard *sb) +static char *prepare_final(struct scoreboard *sb) { int i; const char *final_commit_name = NULL; @@ -2415,10 +2415,10 @@ static const char *prepare_final(struct scoreboard *sb) sb->final = (struct commit *) obj; final_commit_name = revs->pending.objects[i].name; } - return final_commit_name; + return xstrdup_or_null(final_commit_name); } -static const char *prepare_initial(struct scoreboard *sb) +static char *prepare_initial(struct scoreboard *sb) { int i; const char *final_commit_name = NULL; @@ -2445,7 +2445,7 @@ static const char *prepare_initial(struct scoreboard *sb) } if (!final_commit_name) die("No commit to dig down to?"); - return final_commit_name; + return xstrdup(final_commit_name); } static int blame_copy_callback(const struct option *option, const char *arg, int unset) @@ -2489,7 +2489,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) struct origin *o; struct blame_entry *ent = NULL; long dashdash_pos, lno; - const char *final_commit_name = NULL; + char *final_commit_name = NULL; enum object_type type; static struct string_list range_list; @@ -2786,6 +2786,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix) assign_blame(&sb, opt); + free(final_commit_name); + if (incremental) return 0;