From a97262c62f1a31fcc7edf7629d313058bc7d66b5 Mon Sep 17 00:00:00 2001 From: Duy Nguyen Date: Thu, 21 Jan 2016 18:48:44 +0700 Subject: diff: make -O and --output work in subdirectory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- diff.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 46260ed7a1..93e9533389 100644 --- a/diff.c +++ b/diff.c @@ -3695,12 +3695,16 @@ static int parse_ws_error_highlight(struct diff_options *opt, const char *arg) return 1; } -int diff_opt_parse(struct diff_options *options, const char **av, int ac) +int diff_opt_parse(struct diff_options *options, + const char **av, int ac, const char *prefix) { const char *arg = av[0]; const char *optarg; int argcount; + if (!prefix) + prefix = ""; + /* Output format options */ if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch") || opt_arg(arg, 'U', "unified", &options->context)) @@ -3917,7 +3921,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (!strcmp(arg, "--pickaxe-regex")) options->pickaxe_opts |= DIFF_PICKAXE_REGEX; else if ((argcount = short_opt('O', av, &optarg))) { - options->orderfile = optarg; + const char *path = prefix_filename(prefix, strlen(prefix), optarg); + options->orderfile = xstrdup(path); return argcount; } else if ((argcount = parse_long_opt("diff-filter", av, &optarg))) { @@ -3956,9 +3961,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (!strcmp(arg, "--no-function-context")) DIFF_OPT_CLR(options, FUNCCONTEXT); else if ((argcount = parse_long_opt("output", av, &optarg))) { - options->file = fopen(optarg, "w"); + const char *path = prefix_filename(prefix, strlen(prefix), optarg); + options->file = fopen(path, "w"); if (!options->file) - die_errno("Could not open '%s'", optarg); + die_errno("Could not open '%s'", path); options->close_file = 1; return argcount; } else -- cgit v1.3 From a64e6a44c63a965c5bc26242ddd3ed049b42e117 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 13:28:54 -0500 Subject: diff: clarify textconv interface The memory allocation scheme for the textconv interface is a bit tricky, and not well documented. It was originally designed as an internal part of diff.c (matching fill_mmfile), but gradually was made public. Refactoring it is difficult, but we can at least improve the situation by documenting the intended flow and enforcing it with an in-code assertion. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 5 ++++- diff.h | 16 ++++++++++++++++ userdiff.h | 4 ++++ 3 files changed, 24 insertions(+), 1 deletion(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index f62b7f73d8..a09b0b6ae3 100644 --- a/diff.c +++ b/diff.c @@ -4996,7 +4996,7 @@ size_t fill_textconv(struct userdiff_driver *driver, { size_t size; - if (!driver || !driver->textconv) { + if (!driver) { if (!DIFF_FILE_VALID(df)) { *outbuf = ""; return 0; @@ -5007,6 +5007,9 @@ size_t fill_textconv(struct userdiff_driver *driver, return df->size; } + if (!driver->textconv) + die("BUG: fill_textconv called with non-textconv driver"); + if (driver->textconv_cache && df->sha1_valid) { *outbuf = notes_cache_get(driver->textconv_cache, df->sha1, &size); diff --git a/diff.h b/diff.h index 1ac0582228..65a5e78f64 100644 --- a/diff.h +++ b/diff.h @@ -342,10 +342,26 @@ extern void diff_no_index(struct rev_info *, int, const char **, const char *); extern int index_differs_from(const char *def, int diff_flags); +/* + * Fill the contents of the filespec "df", respecting any textconv defined by + * its userdiff driver. The "driver" parameter must come from a + * previous call to get_textconv(), and therefore should either be NULL or have + * textconv enabled. + * + * Note that the memory ownership of the resulting buffer depends on whether + * the driver field is NULL. If it is, then the memory belongs to the filespec + * struct. If it is non-NULL, then "outbuf" points to a newly allocated buffer + * that should be freed by the caller. + */ extern size_t fill_textconv(struct userdiff_driver *driver, struct diff_filespec *df, char **outbuf); +/* + * Look up the userdiff driver for the given filespec, and return it if + * and only if it has textconv enabled (otherwise return NULL). The result + * can be passed to fill_textconv(). + */ extern struct userdiff_driver *get_textconv(struct diff_filespec *one); extern int parse_rename_score(const char **cp_p); diff --git a/userdiff.h b/userdiff.h index 4a7e78ffbc..2ef0ce5452 100644 --- a/userdiff.h +++ b/userdiff.h @@ -23,6 +23,10 @@ int userdiff_config(const char *k, const char *v); struct userdiff_driver *userdiff_find_by_name(const char *name); struct userdiff_driver *userdiff_find_by_path(const char *path); +/* + * Initialize any textconv-related fields in the driver and return it, or NULL + * if it does not have textconv enabled at all. + */ struct userdiff_driver *userdiff_get_textconv(struct userdiff_driver *driver); #endif /* USERDIFF */ -- cgit v1.3 From 96ffc06f72f693d80f05059a1f0e5ca9007d5f1b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:44:32 -0500 Subject: convert trivial cases to FLEX_ARRAY macros Using FLEX_ARRAY macros reduces the amount of manual computation size we have to do. It also ensures we don't overflow size_t, and it makes sure we write the same number of bytes that we allocated. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 4 +--- builtin/blame.c | 4 +--- builtin/help.c | 9 +++------ builtin/mktree.c | 9 +++++---- builtin/reflog.c | 7 ++----- cache-tree.c | 4 +--- combine-diff.c | 4 +--- diff.c | 7 ++----- dir.c | 16 +++------------- hashmap.c | 3 +-- help.c | 6 ++---- log-tree.c | 5 ++--- name-hash.c | 3 +-- ref-filter.c | 6 ++---- refs.c | 6 ++---- refs/files-backend.c | 19 +++++-------------- remote.c | 5 +---- 17 files changed, 35 insertions(+), 82 deletions(-) (limited to 'diff.c') diff --git a/attr.c b/attr.c index c83ec49f18..6537a433da 100644 --- a/attr.c +++ b/attr.c @@ -93,9 +93,7 @@ static struct git_attr *git_attr_internal(const char *name, int len) if (invalid_attr_name(name, len)) return NULL; - a = xmalloc(sizeof(*a) + len + 1); - memcpy(a->name, name, len); - a->name[len] = 0; + FLEX_ALLOC_MEM(a, name, name, len); a->h = hval; a->next = git_attr_hash[pos]; a->attr_nr = attr_nr++; diff --git a/builtin/blame.c b/builtin/blame.c index 4de9e10148..e175d86e56 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -459,13 +459,11 @@ static void queue_blames(struct scoreboard *sb, struct origin *porigin, static struct origin *make_origin(struct commit *commit, const char *path) { struct origin *o; - size_t pathlen = strlen(path) + 1; - o = xcalloc(1, sizeof(*o) + pathlen); + FLEX_ALLOC_STR(o, path, path); o->commit = commit; o->refcnt = 1; o->next = commit->util; commit->util = o; - memcpy(o->path, path, pathlen); /* includes NUL */ return o; } diff --git a/builtin/help.c b/builtin/help.c index 1cd0c1ee44..3c55ce4563 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -171,12 +171,10 @@ static void exec_man_cmd(const char *cmd, const char *page) static void add_man_viewer(const char *name) { struct man_viewer_list **p = &man_viewer_list; - size_t len = strlen(name); while (*p) p = &((*p)->next); - *p = xcalloc(1, (sizeof(**p) + len + 1)); - memcpy((*p)->name, name, len); /* NUL-terminated by xcalloc */ + FLEX_ALLOC_STR(*p, name, name); } static int supported_man_viewer(const char *name, size_t len) @@ -190,9 +188,8 @@ static void do_add_man_viewer_info(const char *name, size_t len, const char *value) { - struct man_viewer_info_list *new = xcalloc(1, sizeof(*new) + len + 1); - - memcpy(new->name, name, len); /* NUL-terminated by xcalloc */ + struct man_viewer_info_list *new; + FLEX_ALLOC_MEM(new, name, name, len); new->info = xstrdup(value); new->next = man_viewer_info_list; man_viewer_info_list = new; diff --git a/builtin/mktree.c b/builtin/mktree.c index a964d6be52..b0aab65353 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -19,16 +19,17 @@ static int alloc, used; static void append_to_tree(unsigned mode, unsigned char *sha1, char *path) { struct treeent *ent; - int len = strlen(path); + size_t len = strlen(path); if (strchr(path, '/')) die("path %s contains slash", path); - ALLOC_GROW(entries, used + 1, alloc); - ent = entries[used++] = xmalloc(sizeof(**entries) + len + 1); + FLEX_ALLOC_MEM(ent, name, path, len); ent->mode = mode; ent->len = len; hashcpy(ent->sha1, sha1); - memcpy(ent->name, path, len+1); + + ALLOC_GROW(entries, used + 1, alloc); + entries[used++] = ent; } static int ent_compare(const void *a_, const void *b_) diff --git a/builtin/reflog.c b/builtin/reflog.c index 9980731ee7..2d46b6482a 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -382,11 +382,9 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus { struct collected_reflog *e; struct collect_reflog_cb *cb = cb_data; - size_t namelen = strlen(ref); - e = xmalloc(sizeof(*e) + namelen + 1); + FLEX_ALLOC_STR(e, reflog, ref); hashcpy(e->sha1, oid->hash); - memcpy(e->reflog, ref, namelen + 1); ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc); cb->e[cb->nr++] = e; return 0; @@ -411,8 +409,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len) ent->pattern[len] == '\0') return ent; - ent = xcalloc(1, sizeof(*ent) + len + 1); - memcpy(ent->pattern, pattern, len); + FLEX_ALLOC_MEM(ent, pattern, pattern, len); *reflog_expire_cfg_tail = ent; reflog_expire_cfg_tail = &(ent->next); return ent; diff --git a/cache-tree.c b/cache-tree.c index a59e6f1e1f..1fbe79a003 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -79,11 +79,9 @@ static struct cache_tree_sub *find_subtree(struct cache_tree *it, ALLOC_GROW(it->down, it->subtree_nr + 1, it->subtree_alloc); it->subtree_nr++; - down = xmalloc(sizeof(*down) + pathlen + 1); + FLEX_ALLOC_MEM(down, name, path, pathlen); down->cache_tree = NULL; down->namelen = pathlen; - memcpy(down->name, path, pathlen); - down->name[pathlen] = 0; if (pos < it->subtree_nr) memmove(it->down + pos + 1, diff --git a/combine-diff.c b/combine-diff.c index 890c4157bd..be09a2b25c 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -319,7 +319,7 @@ static void append_lost(struct sline *sline, int n, const char *line, int len) if (line[len-1] == '\n') len--; - lline = xmalloc(sizeof(*lline) + len + 1); + FLEX_ALLOC_MEM(lline, line, line, len); lline->len = len; lline->next = NULL; lline->prev = sline->plost.lost_tail; @@ -330,8 +330,6 @@ static void append_lost(struct sline *sline, int n, const char *line, int len) sline->plost.lost_tail = lline; sline->plost.len++; lline->parent_map = this_mask; - memcpy(lline->line, line, len); - lline->line[len] = 0; } struct combine_diff_state { diff --git a/diff.c b/diff.c index 2136b6970b..27d14a7d15 100644 --- a/diff.c +++ b/diff.c @@ -2607,12 +2607,9 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, struct diff_filespec *alloc_filespec(const char *path) { - int namelen = strlen(path); - struct diff_filespec *spec = xmalloc(sizeof(*spec) + namelen + 1); + struct diff_filespec *spec; - memset(spec, 0, sizeof(*spec)); - spec->path = (char *)(spec + 1); - memcpy(spec->path, path, namelen+1); + FLEXPTR_ALLOC_STR(spec, path, path); spec->count = 1; spec->is_binary = -1; return spec; diff --git a/dir.c b/dir.c index cb5bff8d8f..c1bc538a56 100644 --- a/dir.c +++ b/dir.c @@ -503,12 +503,7 @@ void add_exclude(const char *string, const char *base, parse_exclude_pattern(&string, &patternlen, &flags, &nowildcardlen); if (flags & EXC_FLAG_MUSTBEDIR) { - char *s; - x = xmalloc(sizeof(*x) + patternlen + 1); - s = (char *)(x+1); - memcpy(s, string, patternlen); - s[patternlen] = '\0'; - x->pattern = s; + FLEXPTR_ALLOC_MEM(x, pattern, string, patternlen); } else { x = xmalloc(sizeof(*x)); x->pattern = string; @@ -625,10 +620,7 @@ static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc, } uc->dir_created++; - d = xmalloc(sizeof(*d) + len + 1); - memset(d, 0, sizeof(*d)); - memcpy(d->name, name, len); - d->name[len] = '\0'; + FLEX_ALLOC_MEM(d, name, name, len); ALLOC_GROW(dir->dirs, dir->dirs_nr + 1, dir->dirs_alloc); memmove(dir->dirs + first + 1, dir->dirs + first, @@ -1167,10 +1159,8 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len) { struct dir_entry *ent; - ent = xmalloc(sizeof(*ent) + len + 1); + FLEX_ALLOC_MEM(ent, name, pathname, len); ent->len = len; - memcpy(ent->name, pathname, len); - ent->name[len] = 0; return ent; } diff --git a/hashmap.c b/hashmap.c index f693839cb4..b10b642229 100644 --- a/hashmap.c +++ b/hashmap.c @@ -256,10 +256,9 @@ const void *memintern(const void *data, size_t len) e = hashmap_get(&map, &key, data); if (!e) { /* not found: create it */ - e = xmallocz(sizeof(struct pool_entry) + len); + FLEX_ALLOC_MEM(e, data, data, len); hashmap_entry_init(e, key.ent.hash); e->len = len; - memcpy(e->data, data, len); hashmap_add(&map, e); } return e->data; diff --git a/help.c b/help.c index d996b34066..19328ea992 100644 --- a/help.c +++ b/help.c @@ -11,11 +11,9 @@ void add_cmdname(struct cmdnames *cmds, const char *name, int len) { - struct cmdname *ent = xmalloc(sizeof(*ent) + len + 1); - + struct cmdname *ent; + FLEX_ALLOC_MEM(ent, name, name, len); ent->len = len; - memcpy(ent->name, name, len); - ent->name[len] = 0; ALLOC_GROW(cmds->names, cmds->cnt + 1, cmds->alloc); cmds->names[cmds->cnt++] = ent; diff --git a/log-tree.c b/log-tree.c index f70a30e127..60f983934d 100644 --- a/log-tree.c +++ b/log-tree.c @@ -77,9 +77,8 @@ int parse_decorate_color_config(const char *var, const char *slot_name, const ch void add_name_decoration(enum decoration_type type, const char *name, struct object *obj) { - int nlen = strlen(name); - struct name_decoration *res = xmalloc(sizeof(*res) + nlen + 1); - memcpy(res->name, name, nlen + 1); + struct name_decoration *res; + FLEX_ALLOC_STR(res, name, name); res->type = type; res->next = add_decoration(&name_decoration, obj, res); } diff --git a/name-hash.c b/name-hash.c index 332ba956e7..6d9f23e932 100644 --- a/name-hash.c +++ b/name-hash.c @@ -55,10 +55,9 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate, dir = find_dir_entry(istate, ce->name, namelen); if (!dir) { /* not found, create it and add to hash table */ - dir = xcalloc(1, sizeof(struct dir_entry) + namelen + 1); + FLEX_ALLOC_MEM(dir, name, ce->name, namelen); hashmap_entry_init(dir, memihash(ce->name, namelen)); dir->namelen = namelen; - strncpy(dir->name, ce->name, namelen); hashmap_add(&istate->dir_hash, dir); /* recursively add missing parent directories */ diff --git a/ref-filter.c b/ref-filter.c index f097176ed9..9ccfc51cea 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1255,10 +1255,8 @@ static struct ref_array_item *new_ref_array_item(const char *refname, const unsigned char *objectname, int flag) { - size_t len = strlen(refname); - struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1); - memcpy(ref->refname, refname, len); - ref->refname[len] = '\0'; + struct ref_array_item *ref; + FLEX_ALLOC_STR(ref, refname, refname); hashcpy(ref->objectname, objectname); ref->flag = flag; diff --git a/refs.c b/refs.c index 1d9e2a7932..2d86445231 100644 --- a/refs.c +++ b/refs.c @@ -761,10 +761,8 @@ void ref_transaction_free(struct ref_transaction *transaction) static struct ref_update *add_update(struct ref_transaction *transaction, const char *refname) { - size_t len = strlen(refname) + 1; - struct ref_update *update = xcalloc(1, sizeof(*update) + len); - - memcpy((char *)update->refname, refname, len); /* includes NUL */ + struct ref_update *update; + FLEX_ALLOC_STR(update, refname, refname); ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc); transaction->updates[transaction->nr++] = update; return update; diff --git a/refs/files-backend.c b/refs/files-backend.c index 3a27f27585..de9af1615c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -199,17 +199,14 @@ static struct ref_entry *create_ref_entry(const char *refname, const unsigned char *sha1, int flag, int check_name) { - int len; struct ref_entry *ref; if (check_name && check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) die("Reference has invalid format: '%s'", refname); - len = strlen(refname) + 1; - ref = xmalloc(sizeof(struct ref_entry) + len); + FLEX_ALLOC_STR(ref, name, refname); hashcpy(ref->u.value.oid.hash, sha1); oidclr(&ref->u.value.peeled); - memcpy(ref->name, refname, len); ref->flag = flag; return ref; } @@ -268,9 +265,7 @@ static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache, int incomplete) { struct ref_entry *direntry; - direntry = xcalloc(1, sizeof(struct ref_entry) + len + 1); - memcpy(direntry->name, dirname, len); - direntry->name[len] = '\0'; + FLEX_ALLOC_MEM(direntry, name, dirname, len); direntry->u.subdir.ref_cache = ref_cache; direntry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0); return direntry; @@ -939,13 +934,10 @@ static void clear_loose_ref_cache(struct ref_cache *refs) */ static struct ref_cache *create_ref_cache(const char *submodule) { - int len; struct ref_cache *refs; if (!submodule) submodule = ""; - len = strlen(submodule) + 1; - refs = xcalloc(1, sizeof(struct ref_cache) + len); - memcpy(refs->name, submodule, len); + FLEX_ALLOC_STR(refs, name, submodule); refs->next = submodule_ref_caches; submodule_ref_caches = refs; return refs; @@ -2191,10 +2183,9 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data) /* Schedule the loose reference for pruning if requested. */ if ((cb->flags & PACK_REFS_PRUNE)) { - int namelen = strlen(entry->name) + 1; - struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen); + struct ref_to_prune *n; + FLEX_ALLOC_STR(n, name, entry->name); hashcpy(n->sha1, entry->u.value.oid.hash); - memcpy(n->name, entry->name, namelen); /* includes NUL */ n->next = cb->ref_to_prune; cb->ref_to_prune = n; } diff --git a/remote.c b/remote.c index 9d34b5a5da..7a8a8a1b4a 100644 --- a/remote.c +++ b/remote.c @@ -2132,16 +2132,13 @@ static int one_local_ref(const char *refname, const struct object_id *oid, { struct ref ***local_tail = cb_data; struct ref *ref; - int len; /* we already know it starts with refs/ to get here */ if (check_refname_format(refname + 5, 0)) return 0; - len = strlen(refname) + 1; - ref = xcalloc(1, sizeof(*ref) + len); + ref = alloc_ref(refname); oidcpy(&ref->new_oid, oid); - memcpy(ref->name, refname, len); **local_tail = ref; *local_tail = &ref->next; return 0; -- cgit v1.3 From b1ddfb9151ca817c30825d1250d565b32c5fc0f5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 22 Feb 2016 17:45:08 -0500 Subject: diff_populate_gitlink: use a strbuf We allocate 100 bytes to hold the "Submodule commit ..." text. This is enough, but it's not immediately obvious that this is the case, and we have to repeat the magic 100 twice. We could get away with xstrfmt here, but we want to know the size, as well, so let's use a real strbuf. And while we're here, we can clean up the logic around size_only. It currently sets and clears the "data" field pointlessly, and leaves the "should_free" flag on even after we have cleared the data. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 27d14a7d15..a70ec6ef1a 100644 --- a/diff.c +++ b/diff.c @@ -2704,21 +2704,21 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int static int diff_populate_gitlink(struct diff_filespec *s, int size_only) { - int len; - char *data = xmalloc(100), *dirty = ""; + struct strbuf buf = STRBUF_INIT; + char *dirty = ""; /* Are we looking at the work tree? */ if (s->dirty_submodule) dirty = "-dirty"; - len = snprintf(data, 100, - "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty); - s->data = data; - s->size = len; - s->should_free = 1; + strbuf_addf(&buf, "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty); + s->size = buf.len; if (size_only) { s->data = NULL; - free(data); + strbuf_release(&buf); + } else { + s->data = strbuf_detach(&buf, NULL); + s->should_free = 1; } return 0; } -- cgit v1.3