From: Junio C Hamano Date: Wed, 16 Jul 2014 18:16:38 +0000 (-0700) Subject: Merge branch 'jk/commit-buffer-length' into maint X-Git-Tag: v2.0.2~4 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/5c18fde0d96cfcbc321caad8f809028b0c63aaeb?hp=-c Merge branch 'jk/commit-buffer-length' into maint A handful of code paths had to read the commit object more than once when showing header fields that are usually not parsed. The internal data structure to keep track of the contents of the commit object has been updated to reduce the need for this double-reading, and to allow the caller find the length of the object. * jk/commit-buffer-length: reuse cached commit buffer when parsing signatures commit: record buffer length in cache commit: convert commit->buffer to a slab commit-slab: provide a static initializer use get_commit_buffer everywhere convert logmsg_reencode to get_commit_buffer use get_commit_buffer to avoid duplicate code use get_cached_commit_buffer where appropriate provide helpers to access the commit buffer provide a helper to set the commit buffer provide a helper to free commit buffer sequencer: use logmsg_reencode in get_message logmsg_reencode: return const buffer do not create "struct commit" with xcalloc commit: push commit_index update into alloc_commit_node alloc: include any-object allocations in alloc_report replace dangerous uses of strbuf_attach commit_tree: take a pointer/len pair rather than a const strbuf --- 5c18fde0d96cfcbc321caad8f809028b0c63aaeb diff --combined builtin/blame.c index d8b276048e,b84e375b5c..ef7cb1d254 --- a/builtin/blame.c +++ b/builtin/blame.c @@@ -1405,7 -1405,7 +1405,7 @@@ static void get_commit_info(struct comm { int len; const char *subject, *encoding; - char *message; + const char *message; commit_info_init(ret); @@@ -1416,7 -1416,7 +1416,7 @@@ &ret->author_time, &ret->author_tz); if (!detailed) { - logmsg_free(message, commit); + unuse_commit_buffer(commit, message); return; } @@@ -1430,7 -1430,7 +1430,7 @@@ else strbuf_addf(&ret->summary, "(%s)", sha1_to_hex(commit->object.sha1)); - logmsg_free(message, commit); + unuse_commit_buffer(commit, message); } /* @@@ -1556,29 -1556,22 +1556,29 @@@ static void assign_blame(struct scorebo static const char *format_time(unsigned long time, const char *tz_str, int show_raw_time) { - static char time_buf[128]; + static struct strbuf time_buf = STRBUF_INIT; + strbuf_reset(&time_buf); if (show_raw_time) { - snprintf(time_buf, sizeof(time_buf), "%lu %s", time, tz_str); + strbuf_addf(&time_buf, "%lu %s", time, tz_str); } else { const char *time_str; - int time_len; + size_t time_width; int tz; tz = atoi(tz_str); time_str = show_date(time, tz, blame_date_mode); - time_len = strlen(time_str); - memcpy(time_buf, time_str, time_len); - memset(time_buf + time_len, ' ', blame_date_width - time_len); + strbuf_addstr(&time_buf, time_str); + /* + * Add space paddings to time_buf to display a fixed width + * string, and use time_width for display width calibration. + */ + for (time_width = utf8_strwidth(time_str); + time_width < blame_date_width; + time_width++) + strbuf_addch(&time_buf, ' '); } - return time_buf; + return time_buf.buf; } #define OUTPUT_ANNOTATE_COMPAT 001 @@@ -2005,6 -1998,18 +2005,18 @@@ static void append_merge_parents(struc strbuf_release(&line); } + /* + * This isn't as simple as passing sb->buf and sb->len, because we + * want to transfer ownership of the buffer to the commit (so we + * must use detach). + */ + static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb) + { + size_t len; + void *buf = strbuf_detach(sb, &len); + set_commit_buffer(c, buf, len); + } + /* * Prepare a dummy commit that represents the work tree (or staged) item. * Note that annotating work tree item never works in the reverse. @@@ -2026,7 -2031,7 +2038,7 @@@ static struct commit *fake_working_tree struct strbuf msg = STRBUF_INIT; time(&now); - commit = xcalloc(1, sizeof(*commit)); + commit = alloc_commit_node(); commit->object.parsed = 1; commit->date = now; commit->object.type = OBJ_COMMIT; @@@ -2053,7 -2058,7 +2065,7 @@@ ident, ident, path, (!contents_from ? path : (!strcmp(contents_from, "-") ? "standard input" : contents_from))); - commit->buffer = strbuf_detach(&msg, NULL); + set_commit_buffer_from_strbuf(commit, &msg); if (!contents_from || strcmp("-", contents_from)) { struct stat st; @@@ -2095,6 -2100,7 +2107,6 @@@ if (strbuf_read(&buf, 0, 0) < 0) die_errno("failed to read from stdin"); } - convert_to_git(path, buf.buf, buf.len, &buf, 0); origin->file.ptr = buf.buf; origin->file.size = buf.len; pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1); @@@ -2337,14 -2343,7 +2349,14 @@@ parse_done blame_date_width = sizeof("2006-10-19"); break; case DATE_RELATIVE: - /* "normal" is used as the fallback for "relative" */ + /* TRANSLATORS: This string is used to tell us the maximum + display width for a relative timestamp in "git blame" + output. For C locale, "4 years, 11 months ago", which + takes 22 places, is the longest among various forms of + relative timestamps, but your language may need more or + fewer display columns. */ + blame_date_width = utf8_strwidth(_("4 years, 11 months ago")) + 1; /* add the null */ + break; case DATE_LOCAL: case DATE_NORMAL: blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700"); diff --combined builtin/commit.c index 12afc42d19,8b85df475b..39cf8976e3 --- a/builtin/commit.c +++ b/builtin/commit.c @@@ -650,8 -650,9 +650,8 @@@ static int prepare_to_commit(const cha } else if (use_message) { char *buffer; buffer = strstr(use_message_buffer, "\n\n"); - if (!use_editor && (!buffer || buffer[2] == '\0')) - die(_("commit has empty message")); - strbuf_add(&sb, buffer + 2, strlen(buffer + 2)); + if (buffer) + strbuf_add(&sb, buffer + 2, strlen(buffer + 2)); hook_arg1 = "commit"; hook_arg2 = use_message; } else if (fixup_message) { @@@ -832,22 -833,8 +832,22 @@@ if (get_sha1(parent, sha1)) commitable = !!active_nr; - else - commitable = index_differs_from(parent, 0); + else { + /* + * Unless the user did explicitly request a submodule + * ignore mode by passing a command line option we do + * not ignore any changed submodule SHA-1s when + * comparing index and parent, no matter what is + * configured. Otherwise we won't commit any + * submodules which were manually staged, which would + * be really confusing. + */ + int diff_flags = DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG; + if (ignore_submodule_arg && + !strcmp(ignore_submodule_arg, "all")) + diff_flags |= DIFF_OPT_IGNORE_SUBMODULES; + commitable = index_differs_from(parent, diff_flags); + } } strbuf_release(&committer_ident); @@@ -1672,8 -1659,8 +1672,8 @@@ int cmd_commit(int argc, const char **a append_merge_tag_headers(parents, &tail); } - if (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1, - author_ident.buf, sign_commit, extra)) { + if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1, + parents, sha1, author_ident.buf, sign_commit, extra)) { rollback_index_files(); die(_("failed to write commit object")); } diff --combined builtin/index-pack.c index 18f57de58b,459b9f07bb..8b3bd29dbc --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@@ -40,13 -40,17 +40,13 @@@ struct base_data int ofs_first, ofs_last; }; -#if !defined(NO_PTHREADS) && defined(NO_THREAD_SAFE_PREAD) -/* pread() emulation is not thread-safe. Disable threading. */ -#define NO_PTHREADS -#endif - struct thread_local { #ifndef NO_PTHREADS pthread_t thread; #endif struct base_data *base_cache; size_t base_cache_used; + int pack_fd; }; /* @@@ -87,8 -91,7 +87,8 @@@ static off_t consumed_bytes static unsigned deepest_delta; static git_SHA_CTX input_ctx; static uint32_t input_crc32; -static int input_fd, output_fd, pack_fd; +static int input_fd, output_fd; +static const char *curr_pack; #ifndef NO_PTHREADS @@@ -131,7 -134,6 +131,7 @@@ static inline void unlock_mutex(pthread */ static void init_thread(void) { + int i; init_recursive_mutex(&read_mutex); pthread_mutex_init(&counter_mutex, NULL); pthread_mutex_init(&work_mutex, NULL); @@@ -139,18 -141,11 +139,18 @@@ pthread_mutex_init(&deepest_delta_mutex, NULL); pthread_key_create(&key, NULL); thread_data = xcalloc(nr_threads, sizeof(*thread_data)); + for (i = 0; i < nr_threads; i++) { + thread_data[i].pack_fd = open(curr_pack, O_RDONLY); + if (thread_data[i].pack_fd == -1) + die_errno(_("unable to open %s"), curr_pack); + } + threads_active = 1; } static void cleanup_thread(void) { + int i; if (!threads_active) return; threads_active = 0; @@@ -159,8 -154,6 +159,8 @@@ pthread_mutex_destroy(&work_mutex); if (show_stat) pthread_mutex_destroy(&deepest_delta_mutex); + for (i = 0; i < nr_threads; i++) + close(thread_data[i].pack_fd); pthread_key_delete(key); free(thread_data); } @@@ -207,13 -200,8 +207,13 @@@ static unsigned check_object(struct obj if (!(obj->flags & FLAG_CHECKED)) { unsigned long size; int type = sha1_object_info(obj->sha1, &size); - if (type != obj->type || type <= 0) - die(_("object of unexpected type")); + if (type <= 0) + die(_("did not receive expected object %s"), + sha1_to_hex(obj->sha1)); + if (type != obj->type) + die(_("object %s: expected type %s, found %s"), + sha1_to_hex(obj->sha1), + typename(obj->type), typename(type)); obj->flags |= FLAG_CHECKED; return 1; } @@@ -300,13 -288,13 +300,13 @@@ static const char *open_pack_file(cons output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR, 0600); if (output_fd < 0) die_errno(_("unable to create '%s'"), pack_name); - pack_fd = output_fd; + nothread_data.pack_fd = output_fd; } else { input_fd = open(pack_name, O_RDONLY); if (input_fd < 0) die_errno(_("cannot open packfile '%s'"), pack_name); output_fd = -1; - pack_fd = input_fd; + nothread_data.pack_fd = input_fd; } git_SHA1_Init(&input_ctx); return pack_name; @@@ -554,7 -542,7 +554,7 @@@ static void *unpack_data(struct object_ do { ssize_t n = (len < 64*1024) ? len : 64*1024; - n = pread(pack_fd, inbuf, n, from); + n = xpread(get_thread_data()->pack_fd, inbuf, n, from); if (n < 0) die_errno(_("cannot pread pack file")); if (!n) @@@ -786,7 -774,8 +786,8 @@@ static void sha1_object(const void *dat } if (obj->type == OBJ_COMMIT) { struct commit *commit = (struct commit *) obj; - commit->buffer = NULL; + if (detach_commit_buffer(commit, NULL) != data) + die("BUG: parse_object_buffer transmogrified our buffer"); } obj->flags |= FLAG_CHECKED; } @@@ -1502,7 -1491,7 +1503,7 @@@ static void show_pack_info(int stat_onl int cmd_index_pack(int argc, const char **argv, const char *prefix) { int i, fix_thin_pack = 0, verify = 0, stat_only = 0; - const char *curr_pack, *curr_index; + const char *curr_index; const char *index_name = NULL, *pack_name = NULL; const char *keep_name = NULL, *keep_msg = NULL; char *index_name_buf = NULL, *keep_name_buf = NULL; diff --combined builtin/log.c index 3b6a6bbadd,c599eacf72..4f678136d1 --- a/builtin/log.c +++ b/builtin/log.c @@@ -158,9 -158,13 +158,9 @@@ static void cmd_log_init_finish(int arg if (rev->show_notes) init_display_notes(&rev->notes_opt); - if (rev->diffopt.pickaxe || rev->diffopt.filter) + if (rev->diffopt.pickaxe || rev->diffopt.filter || + DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) rev->always_show_header = 0; - if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) { - rev->always_show_header = 0; - if (rev->diffopt.pathspec.nr != 1) - usage("git logs can only follow renames on one pathname at a time"); - } if (source) rev->show_source = 1; @@@ -345,8 -349,7 +345,7 @@@ static int cmd_log_walk(struct rev_inf rev->max_count++; if (!rev->reflog_info) { /* we allow cycles in reflog ancestry */ - free(commit->buffer); - commit->buffer = NULL; + free_commit_buffer(commit); } free_commit_list(commit->parents); commit->parents = NULL; @@@ -915,9 -918,12 +914,12 @@@ static void make_cover_letter(struct re log_write_email_headers(rev, head, &pp.subject, &pp.after_subject, &need_8bit_cte); - for (i = 0; !need_8bit_cte && i < nr; i++) - if (has_non_ascii(list[i]->buffer)) + for (i = 0; !need_8bit_cte && i < nr; i++) { + const char *buf = get_commit_buffer(list[i], NULL); + if (has_non_ascii(buf)) need_8bit_cte = 1; + unuse_commit_buffer(list[i], buf); + } if (!branch_name) branch_name = find_branch_name(rev); @@@ -1504,8 -1510,7 +1506,7 @@@ int cmd_format_patch(int argc, const ch reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, quiet)) die(_("Failed to create output files")); shown = log_tree_commit(&rev, commit); - free(commit->buffer); - commit->buffer = NULL; + free_commit_buffer(commit); /* We put one extra blank line between formatted * patches and this flag is used by log-tree code diff --combined merge-recursive.c index cab16fafb5,a9ab328923..d38a3b2eb5 --- a/merge-recursive.c +++ b/merge-recursive.c @@@ -40,7 -40,7 +40,7 @@@ static struct tree *shift_tree_object(s static struct commit *make_virtual_commit(struct tree *tree, const char *comment) { - struct commit *commit = xcalloc(1, sizeof(struct commit)); + struct commit *commit = alloc_commit_node(); struct merge_remote_desc *desc = xmalloc(sizeof(*desc)); desc->name = comment; @@@ -190,9 -190,11 +190,11 @@@ static void output_commit_title(struct printf(_("(bad commit)\n")); else { const char *title; - int len = find_commit_subject(commit->buffer, &title); + const char *msg = get_commit_buffer(commit, NULL); + int len = find_commit_subject(msg, &title); if (len) printf("%.*s\n", len, title); + unuse_commit_buffer(commit, msg); } } } @@@ -589,12 -591,6 +591,12 @@@ static int remove_file(struct merge_opt return -1; } if (update_working_directory) { + if (ignore_case) { + struct cache_entry *ce; + ce = cache_file_exists(path, strlen(path), ignore_case); + if (ce && ce_stage(ce) == 0) + return 0; + } if (remove_path(path)) return -1; } diff --combined pretty.c index 5c02b29ceb,b9fceedbb9..32099632f6 --- a/pretty.c +++ b/pretty.c @@@ -274,7 -274,7 +274,7 @@@ static void add_rfc822_quoted(struct st enum rfc2047_type { RFC2047_SUBJECT, - RFC2047_ADDRESS, + RFC2047_ADDRESS }; static int is_rfc2047_special(char ch, enum rfc2047_type type) @@@ -606,29 -606,16 +606,16 @@@ static char *replace_encoding_header(ch return strbuf_detach(&tmp, NULL); } - char *logmsg_reencode(const struct commit *commit, - char **commit_encoding, - const char *output_encoding) + const char *logmsg_reencode(const struct commit *commit, + char **commit_encoding, + const char *output_encoding) { static const char *utf8 = "UTF-8"; const char *use_encoding; char *encoding; - char *msg = commit->buffer; + const char *msg = get_commit_buffer(commit, NULL); char *out; - if (!msg) { - enum object_type type; - unsigned long size; - - msg = read_sha1_file(commit->object.sha1, &type, &size); - if (!msg) - die("Cannot read commit object %s", - sha1_to_hex(commit->object.sha1)); - if (type != OBJ_COMMIT) - die("Expected commit for '%s', got %s", - sha1_to_hex(commit->object.sha1), typename(type)); - } - if (!output_encoding || !*output_encoding) { if (commit_encoding) *commit_encoding = @@@ -652,12 -639,13 +639,13 @@@ * Otherwise, we still want to munge the encoding header in the * result, which will be done by modifying the buffer. If we * are using a fresh copy, we can reuse it. But if we are using - * the cached copy from commit->buffer, we need to duplicate it - * to avoid munging commit->buffer. + * the cached copy from get_commit_buffer, we need to duplicate it + * to avoid munging the cached copy. */ - out = msg; - if (out == commit->buffer) - out = xstrdup(out); + if (msg == get_cached_commit_buffer(commit, NULL)) + out = xstrdup(msg); + else + out = (char *)msg; } else { /* @@@ -667,8 -655,8 +655,8 @@@ * copy, we can free it. */ out = reencode_string(msg, output_encoding, use_encoding); - if (out && msg != commit->buffer) - free(msg); + if (out) + unuse_commit_buffer(commit, msg); } /* @@@ -687,12 -675,6 +675,6 @@@ return out ? out : msg; } - void logmsg_free(char *msg, const struct commit *commit) - { - if (msg != commit->buffer) - free(msg); - } - static int mailmap_name(const char **email, size_t *email_len, const char **name, size_t *name_len) { @@@ -796,7 -778,7 +778,7 @@@ struct format_commit_context struct signature_check signature_check; enum flush_type flush_type; enum trunc_type truncate; - char *message; + const char *message; char *commit_encoding; size_t width, indent1, indent2; int auto_color; @@@ -1506,18 -1488,13 +1488,18 @@@ void format_commit_message(const struc context.commit = commit; context.pretty_ctx = pretty_ctx; context.wrap_start = sb->len; + /* + * convert a commit message to UTF-8 first + * as far as 'format_commit_item' assumes it in UTF-8 + */ context.message = logmsg_reencode(commit, &context.commit_encoding, - output_enc); + utf8); strbuf_expand(sb, format, format_commit_item, &context); rewrap_message_tail(sb, &context, 0, 0, 0); + /* then convert a commit message to an actual output encoding */ if (output_enc) { if (same_encoding(utf8, output_enc)) output_enc = NULL; @@@ -1536,7 -1513,7 +1518,7 @@@ } free(context.commit_encoding); - logmsg_free(context.message, commit); + unuse_commit_buffer(commit, context.message); free(context.signature_check.gpg_output); free(context.signature_check.signer); } @@@ -1705,7 -1682,7 +1687,7 @@@ void pretty_print_commit(struct pretty_ unsigned long beginning_of_body; int indent = 4; const char *msg; - char *reencoded; + const char *reencoded; const char *encoding; int need_8bit_cte = pp->need_8bit_cte; @@@ -1772,7 -1749,7 +1754,7 @@@ if (pp->fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body) strbuf_addch(sb, '\n'); - logmsg_free(reencoded, commit); + unuse_commit_buffer(commit, reencoded); } void pp_commit_easy(enum cmit_fmt fmt, const struct commit *commit, diff --combined revision.c index 8351e794df,1cc91e5911..2571ada6bf --- a/revision.c +++ b/revision.c @@@ -1633,7 -1633,6 +1633,7 @@@ static int handle_revision_opt(struct r !strcmp(arg, "--reflog") || !strcmp(arg, "--not") || !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk") || !strcmp(arg, "--bisect") || starts_with(arg, "--glob=") || + starts_with(arg, "--exclude=") || starts_with(arg, "--branches=") || starts_with(arg, "--tags=") || starts_with(arg, "--remotes=") || starts_with(arg, "--no-walk=")) { @@@ -1649,10 -1648,8 +1649,10 @@@ revs->skip_count = atoi(optarg); return argcount; } else if ((*arg == '-') && isdigit(arg[1])) { - /* accept -, like traditional "head" */ - revs->max_count = atoi(arg + 1); + /* accept -, like traditional "head" */ + if (strtol_i(arg + 1, 10, &revs->max_count) < 0 || + revs->max_count < 0) + die("'%s': not a non-negative integer", arg + 1); revs->no_walk = 0; } else if (!strcmp(arg, "-n")) { if (argc <= 1) @@@ -2791,7 -2788,7 +2791,7 @@@ static int commit_match(struct commit * { int retval; const char *encoding; - char *message; + const char *message; struct strbuf buf = STRBUF_INIT; if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list) @@@ -2833,14 -2830,21 +2833,21 @@@ format_display_notes(commit->object.sha1, &buf, encoding, 1); } - /* Find either in the original commit message, or in the temporary */ + /* + * Find either in the original commit message, or in the temporary. + * Note that we cast away the constness of "message" here. It is + * const because it may come from the cached commit buffer. That's OK, + * because we know that it is modifiable heap memory, and that while + * grep_buffer may modify it for speed, it will restore any + * changes before returning. + */ if (buf.len) retval = grep_buffer(&opt->grep_filter, buf.buf, buf.len); else retval = grep_buffer(&opt->grep_filter, - message, strlen(message)); + (char *)message, strlen(message)); strbuf_release(&buf); - logmsg_free(message, commit); + unuse_commit_buffer(commit, message); return retval; }