From: Junio C Hamano Date: Fri, 26 Feb 2016 21:37:15 +0000 (-0800) Subject: Merge branch 'jk/more-comments-on-textconv' X-Git-Tag: v2.8.0-rc0~20 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/3ed26a44b3415f5761bdab21f823d964c20a2824?hp=-c Merge branch 'jk/more-comments-on-textconv' The memory ownership rule of fill_textconv() API, which was a bit tricky, has been documented a bit better. * jk/more-comments-on-textconv: diff: clarify textconv interface --- 3ed26a44b3415f5761bdab21f823d964c20a2824 diff --combined diff.c index 2136b6970b,a09b0b6ae3..a088e269b4 --- a/diff.c +++ b/diff.c @@@ -2,7 -2,6 +2,7 @@@ * Copyright (C) 2005 Junio C Hamano */ #include "cache.h" +#include "tempfile.h" #include "quote.h" #include "diff.h" #include "diffcore.h" @@@ -13,7 -12,7 +13,7 @@@ #include "run-command.h" #include "utf8.h" #include "userdiff.h" -#include "sigchain.h" +#include "submodule-config.h" #include "submodule.h" #include "ll-merge.h" #include "string-list.h" @@@ -309,26 -308,11 +309,26 @@@ static const char *external_diff(void return external_diff_cmd; } +/* + * Keep track of files used for diffing. Sometimes such an entry + * refers to a temporary file, sometimes to an existing file, and + * sometimes to "/dev/null". + */ static struct diff_tempfile { - const char *name; /* filename external diff should read from */ - char hex[41]; + /* + * filename external diff should read from, or NULL if this + * entry is currently not in use: + */ + const char *name; + + char hex[GIT_SHA1_HEXSZ + 1]; char mode[10]; - char tmp_path[PATH_MAX]; + + /* + * If this diff_tempfile instance refers to a temporary file, + * this tempfile object is used to manage its lifetime. + */ + struct tempfile tempfile; } diff_temp[2]; typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len); @@@ -494,59 -478,26 +494,59 @@@ static int new_blank_line_at_eof(struc return ws_blank_line(line, len, ecbdata->ws_rule); } -static void emit_add_line(const char *reset, - struct emit_callback *ecbdata, - const char *line, int len) +static void emit_line_checked(const char *reset, + struct emit_callback *ecbdata, + const char *line, int len, + enum color_diff color, + unsigned ws_error_highlight, + char sign) { - const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE); - const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW); + const char *set = diff_get_color(ecbdata->color_diff, color); + const char *ws = NULL; - if (!*ws) - emit_line_0(ecbdata->opt, set, reset, '+', line, len); - else if (new_blank_line_at_eof(ecbdata, line, len)) + if (ecbdata->opt->ws_error_highlight & ws_error_highlight) { + ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE); + if (!*ws) + ws = NULL; + } + + if (!ws) + emit_line_0(ecbdata->opt, set, reset, sign, line, len); + else if (sign == '+' && new_blank_line_at_eof(ecbdata, line, len)) /* Blank line at EOF - paint '+' as well */ - emit_line_0(ecbdata->opt, ws, reset, '+', line, len); + emit_line_0(ecbdata->opt, ws, reset, sign, line, len); else { /* Emit just the prefix, then the rest. */ - emit_line_0(ecbdata->opt, set, reset, '+', "", 0); + emit_line_0(ecbdata->opt, set, reset, sign, "", 0); ws_check_emit(line, len, ecbdata->ws_rule, ecbdata->opt->file, set, reset, ws); } } +static void emit_add_line(const char *reset, + struct emit_callback *ecbdata, + const char *line, int len) +{ + emit_line_checked(reset, ecbdata, line, len, + DIFF_FILE_NEW, WSEH_NEW, '+'); +} + +static void emit_del_line(const char *reset, + struct emit_callback *ecbdata, + const char *line, int len) +{ + emit_line_checked(reset, ecbdata, line, len, + DIFF_FILE_OLD, WSEH_OLD, '-'); +} + +static void emit_context_line(const char *reset, + struct emit_callback *ecbdata, + const char *line, int len) +{ + emit_line_checked(reset, ecbdata, line, len, + DIFF_CONTEXT, WSEH_CONTEXT, ' '); +} + static void emit_hunk_header(struct emit_callback *ecbdata, const char *line, int len) { @@@ -613,16 -564,25 +613,16 @@@ static struct diff_tempfile *claim_diff die("BUG: diff is failing to clean up its tempfiles"); } -static int remove_tempfile_installed; - static void remove_tempfile(void) { int i; for (i = 0; i < ARRAY_SIZE(diff_temp); i++) { - if (diff_temp[i].name == diff_temp[i].tmp_path) - unlink_or_warn(diff_temp[i].name); + if (is_tempfile_active(&diff_temp[i].tempfile)) + delete_tempfile(&diff_temp[i].tempfile); diff_temp[i].name = NULL; } } -static void remove_tempfile_on_signal(int signo) -{ - remove_tempfile(); - sigchain_pop(signo); - raise(signo); -} - static void print_line_count(FILE *file, int count) { switch (count) { @@@ -643,6 -603,7 +643,6 @@@ static void emit_rewrite_lines(struct e { const char *endp = NULL; static const char *nneof = " No newline at end of file\n"; - const char *old = diff_get_color(ecb->color_diff, DIFF_FILE_OLD); const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET); while (0 < size) { @@@ -652,7 -613,8 +652,7 @@@ len = endp ? (endp - data + 1) : size; if (prefix != '+') { ecb->lno_in_preimage++; - emit_line_0(ecb->opt, old, reset, '-', - data, len); + emit_del_line(reset, ecb, data, len); } else { ecb->lno_in_postimage++; emit_add_line(reset, ecb, data, len); @@@ -1289,27 -1251,17 +1289,27 @@@ static void fn_out_consume(void *priv, return; } - if (line[0] != '+') { - const char *color = - diff_get_color(ecbdata->color_diff, - line[0] == '-' ? DIFF_FILE_OLD : DIFF_CONTEXT); - ecbdata->lno_in_preimage++; - if (line[0] == ' ') - ecbdata->lno_in_postimage++; - emit_line(ecbdata->opt, color, reset, line, len); - } else { + switch (line[0]) { + case '+': ecbdata->lno_in_postimage++; emit_add_line(reset, ecbdata, line + 1, len - 1); + break; + case '-': + ecbdata->lno_in_preimage++; + emit_del_line(reset, ecbdata, line + 1, len - 1); + break; + case ' ': + ecbdata->lno_in_postimage++; + ecbdata->lno_in_preimage++; + emit_context_line(reset, ecbdata, line + 1, len - 1); + break; + default: + /* incomplete line at the end */ + ecbdata->lno_in_preimage++; + emit_line(ecbdata->opt, + diff_get_color(ecbdata->color_diff, DIFF_CONTEXT), + reset, line, len); + break; } } @@@ -2869,7 -2821,8 +2869,7 @@@ static void prep_temp_blob(const char * strbuf_addstr(&template, "XXXXXX_"); strbuf_addstr(&template, base); - fd = git_mkstemps(temp->tmp_path, PATH_MAX, template.buf, - strlen(base) + 1); + fd = mks_tempfile_ts(&temp->tempfile, template.buf, strlen(base) + 1); if (fd < 0) die_errno("unable to create temp-file"); if (convert_to_working_tree(path, @@@ -2879,10 -2832,11 +2879,10 @@@ } if (write_in_full(fd, blob, size) != size) die_errno("unable to write temp-file"); - close(fd); - temp->name = temp->tmp_path; - strcpy(temp->hex, sha1_to_hex(sha1)); - temp->hex[40] = 0; - sprintf(temp->mode, "%06o", mode); + close_tempfile(&temp->tempfile); + temp->name = get_tempfile_path(&temp->tempfile); + sha1_to_hex_r(temp->hex, sha1); + xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode); strbuf_release(&buf); strbuf_release(&template); free(path_dup); @@@ -2899,11 -2853,17 +2899,11 @@@ static struct diff_tempfile *prepare_te * a '+' entry produces this for file-1. */ temp->name = "/dev/null"; - strcpy(temp->hex, "."); - strcpy(temp->mode, "."); + xsnprintf(temp->hex, sizeof(temp->hex), "."); + xsnprintf(temp->mode, sizeof(temp->mode), "."); return temp; } - if (!remove_tempfile_installed) { - atexit(remove_tempfile); - sigchain_push_common(remove_tempfile_on_signal); - remove_tempfile_installed = 1; - } - if (!S_ISGITLINK(one->mode) && (!one->sha1_valid || reuse_worktree_file(name, one->sha1, 1))) { @@@ -2928,16 -2888,16 +2928,16 @@@ /* we can borrow from the file in the work tree */ temp->name = name; if (!one->sha1_valid) - strcpy(temp->hex, sha1_to_hex(null_sha1)); + sha1_to_hex_r(temp->hex, null_sha1); else - strcpy(temp->hex, sha1_to_hex(one->sha1)); + sha1_to_hex_r(temp->hex, one->sha1); /* Even though we may sometimes borrow the * contents from the work tree, we always want * one->mode. mode is trustworthy even when * !(one->sha1_valid), as long as * DIFF_FILE_VALID(one). */ - sprintf(temp->mode, "%06o", one->mode); + xsnprintf(temp->mode, sizeof(temp->mode), "%06o", one->mode); } return temp; } @@@ -3267,7 -3227,6 +3267,7 @@@ void diff_setup(struct diff_options *op options->rename_limit = -1; options->dirstat_permille = diff_dirstat_permille_default; options->context = diff_context_default; + options->ws_error_highlight = WSEH_NEW; DIFF_OPT_SET(options, RENAME_EMPTY); /* pathchange left =NULL by default */ @@@ -3654,55 -3613,12 +3654,55 @@@ static void enable_patch_output(int *fm *fmt |= DIFF_FORMAT_PATCH; } -int diff_opt_parse(struct diff_options *options, const char **av, int ac) +static int parse_one_token(const char **arg, const char *token) +{ + const char *rest; + if (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) { + *arg = rest; + return 1; + } + return 0; +} + +static int parse_ws_error_highlight(struct diff_options *opt, const char *arg) +{ + const char *orig_arg = arg; + unsigned val = 0; + while (*arg) { + if (parse_one_token(&arg, "none")) + val = 0; + else if (parse_one_token(&arg, "default")) + val = WSEH_NEW; + else if (parse_one_token(&arg, "all")) + val = WSEH_NEW | WSEH_OLD | WSEH_CONTEXT; + else if (parse_one_token(&arg, "new")) + val |= WSEH_NEW; + else if (parse_one_token(&arg, "old")) + val |= WSEH_OLD; + else if (parse_one_token(&arg, "context")) + val |= WSEH_CONTEXT; + else { + error("unknown value after ws-error-highlight=%.*s", + (int)(arg - orig_arg), orig_arg); + return 0; + } + if (*arg) + arg++; + } + opt->ws_error_highlight = val; + return 1; +} + +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)) @@@ -3827,10 -3743,9 +3827,10 @@@ DIFF_OPT_SET(options, FIND_COPIES_HARDER); else if (!strcmp(arg, "--follow")) DIFF_OPT_SET(options, FOLLOW_RENAMES); - else if (!strcmp(arg, "--no-follow")) + else if (!strcmp(arg, "--no-follow")) { DIFF_OPT_CLR(options, FOLLOW_RENAMES); - else if (!strcmp(arg, "--color")) + DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES); + } else if (!strcmp(arg, "--color")) options->use_color = 1; else if (skip_prefix(arg, "--color=", &arg)) { int value = git_config_colorbool(NULL, arg); @@@ -3895,8 -3810,6 +3895,8 @@@ DIFF_OPT_SET(options, SUBMODULE_LOG); else if (skip_prefix(arg, "--submodule=", &arg)) return parse_submodule_opt(options, arg); + else if (skip_prefix(arg, "--ws-error-highlight=", &arg)) + return parse_ws_error_highlight(options, arg); /* misc options */ else if (!strcmp(arg, "-z")) @@@ -3919,8 -3832,7 +3919,8 @@@ 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))) { @@@ -3959,10 -3871,9 +3959,10 @@@ 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 @@@ -4089,9 -4000,9 +4089,9 @@@ const char *diff_unique_abbrev(const un if (abblen < 37) { static char hex[41]; if (len < abblen && abblen <= len + 2) - sprintf(hex, "%s%.*s", abbrev, len+3-abblen, ".."); + xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); else - sprintf(hex, "%s...", abbrev); + xsnprintf(hex, sizeof(hex), "%s...", abbrev); return hex; } return sha1_to_hex(sha1); @@@ -5085,7 -4996,7 +5085,7 @@@ size_t fill_textconv(struct userdiff_dr { size_t size; - if (!driver || !driver->textconv) { + if (!driver) { if (!DIFF_FILE_VALID(df)) { *outbuf = ""; return 0; @@@ -5096,6 -5007,9 +5096,9 @@@ 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 --combined diff.h index 70b2d70d64,65a5e78f64..4505b4d91d --- a/diff.h +++ b/diff.h @@@ -6,7 -6,6 +6,7 @@@ #include "tree-walk.h" #include "pathspec.h" +#include "object.h" struct rev_info; struct diff_options; @@@ -91,7 -90,6 +91,7 @@@ typedef struct strbuf *(*diff_prefix_fn #define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28) #define DIFF_OPT_FUNCCONTEXT (1 << 29) #define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30) +#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1U << 31) #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag) #define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag) @@@ -139,11 -137,6 +139,11 @@@ struct diff_options int dirstat_permille; int setup; int abbrev; +/* white-space error highlighting */ +#define WSEH_NEW 1 +#define WSEH_CONTEXT 2 +#define WSEH_OLD 4 + unsigned ws_error_highlight; const char *prefix; int prefix_length; const char *stat_sep; @@@ -214,11 -207,11 +214,11 @@@ struct combine_diff_path struct combine_diff_path *next; char *path; unsigned int mode; - unsigned char sha1[20]; + struct object_id oid; struct combine_diff_parent { char status; unsigned int mode; - unsigned char sha1[20]; + struct object_id oid; } parent[FLEX_ARRAY]; }; #define combine_diff_path_size(n, l) \ @@@ -268,7 -261,7 +268,7 @@@ extern int parse_long_opt(const char *o extern int git_diff_basic_config(const char *var, const char *value, void *cb); extern int git_diff_ui_config(const char *var, const char *value, void *cb); extern void diff_setup(struct diff_options *); -extern int diff_opt_parse(struct diff_options *, const char **, int); +extern int diff_opt_parse(struct diff_options *, const char **, int, const char *); extern void diff_setup_done(struct diff_options *); #define DIFF_DETECT_RENAME 1 @@@ -345,14 -338,30 +345,30 @@@ extern int diff_flush_patch_id(struct d extern int diff_result_code(struct diff_options *, int); -extern void diff_no_index(struct rev_info *, int, const char **, const char *); +extern void diff_no_index(struct rev_info *, int, 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);