Am 02.05.2010 17:10, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> writes:
>> diff --git a/merge-file.c b/merge-file.c
>> index c336c93..db4d0d5 100644
>> --- a/merge-file.c
>> +++ b/merge-file.c
>> @@ -66,7 +66,7 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
>> xdemitcb_t ecb;
>>
>> memset(&xpp, 0, sizeof(xpp));
>> - xpp.flags = XDF_NEED_MINIMAL;
>> + xpp.flags = 0;
>> memset(&xecfg, 0, sizeof(xecfg));
>> xecfg.ctxlen = 3;
>> xecfg.flags = XDL_EMIT_COMMON;
>
> When I wrote the message you are responding to, I tried to decide which is
> better, to replace the assigned value like your patch does, or to remove
> the assignment altogether as we have memset(&xpp, 0, sizeof(xpp)). And I
> was somewhat torn.
>
> While it expresses what the patch wants to do a lot clearer (and it also
> marks the places the "later patch" needs to touch), the resulting code
> becomes slightly harder to read, because future readers of the code are
> left with an obvious "why do we assign 0 after clearing the whole thing?
> is there anything subtle going on?" unanswered.
Well, I didn't do that because of a mix of laziness and caution. A
mechanical replacement is much less likely to introduce bugs..
But when I take a closer look at the surrounding code, I can't help but
ask if the flags really have be passed in such a complicated way.
How about the following, which makes xdi_diff*() take a simple flag
parameter instead, moving the code to handle xpparam_t into
xdiff-interface.c, which seems to be the proper place for it?
René
---
builtin/blame.c | 10 ++--------
builtin/merge-tree.c | 4 +---
builtin/rerere.c | 5 +----
combine-diff.c | 5 +----
diff.c | 25 +++++--------------------
merge-file.c | 5 +----
xdiff-interface.c | 18 +++++++++++-------
xdiff-interface.h | 8 ++++----
8 files changed, 26 insertions(+), 54 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 8deeee1..1c1c9e4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -734,7 +734,6 @@ static int pass_blame_to_parent(struct scoreboard *sb,
int last_in_target;
mmfile_t file_p, file_o;
struct blame_chunk_cb_data d = { sb, target, parent, 0, 0 };
- xpparam_t xpp;
xdemitconf_t xecfg;
last_in_target = find_last_in_target(sb, target);
@@ -745,11 +744,9 @@ static int pass_blame_to_parent(struct scoreboard *sb,
fill_origin_blob(target, &file_o);
num_get_patch++;
- memset(&xpp, 0, sizeof(xpp));
- xpp.flags = xdl_opts;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 0;
- xdi_diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, &xpp, &xecfg);
+ xdi_diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, &xecfg, xdl_opts);
/* The rest (i.e. anything after tlno) are the same as the parent */
blame_chunk(sb, d.tlno, d.plno, last_in_target, target, parent);
@@ -876,7 +873,6 @@ static void find_copy_in_blob(struct scoreboard *sb,
int cnt;
mmfile_t file_o;
struct handle_split_cb_data d = { sb, ent, parent, split, 0, 0 };
- xpparam_t xpp;
xdemitconf_t xecfg;
/*
@@ -896,12 +892,10 @@ static void find_copy_in_blob(struct scoreboard *sb,
* file_o is a part of final image we are annotating.
* file_p partially may match that image.
*/
- memset(&xpp, 0, sizeof(xpp));
- xpp.flags = xdl_opts;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 1;
memset(split, 0, sizeof(struct blame_entry [3]));
- xdi_diff_hunks(file_p, &file_o, handle_split_cb, &d, &xpp, &xecfg);
+ xdi_diff_hunks(file_p, &file_o, handle_split_cb, &d, &xecfg, xdl_opts);
/* remainder, if any, all match the preimage */
handle_split(sb, ent, d.tlno, d.plno, ent->num_lines, parent, split);
}
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index fc00d79..d95cd1c 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -102,11 +102,9 @@ static void show_diff(struct merge_list *entry)
{
unsigned long size;
mmfile_t src, dst;
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
- xpp.flags = 0;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 3;
ecb.outf = show_outf;
@@ -120,7 +118,7 @@ static void show_diff(struct merge_list *entry)
if (!dst.ptr)
size = 0;
dst.size = size;
- xdi_diff(&src, &dst, &xpp, &xecfg, &ecb);
+ xdi_diff(&src, &dst, &xecfg, &ecb, 0);
free(src.ptr);
free(dst.ptr);
}
diff --git a/builtin/rerere.c b/builtin/rerere.c
index 0048f9e..43b75fa 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -78,7 +78,6 @@ static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
static int diff_two(const char *file1, const char *label1,
const char *file2, const char *label2)
{
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
mmfile_t minus, plus;
@@ -88,12 +87,10 @@ static int diff_two(const char *file1, const char *label1,
printf("--- a/%s\n+++ b/%s\n", label1, label2);
fflush(stdout);
- memset(&xpp, 0, sizeof(xpp));
- xpp.flags = 0;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 3;
ecb.outf = outf;
- xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);
+ xdi_diff(&minus, &plus, &xecfg, &ecb, 0);
free(minus.ptr);
free(plus.ptr);
diff --git a/combine-diff.c b/combine-diff.c
index 29779be..5b29226 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -208,7 +208,6 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL << n);
- xpparam_t xpp;
xdemitconf_t xecfg;
mmfile_t parent_file;
xdemitcb_t ecb;
@@ -220,8 +219,6 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
parent_file.ptr = grab_blob(parent, mode, &sz);
parent_file.size = sz;
- memset(&xpp, 0, sizeof(xpp));
- xpp.flags = 0;
memset(&xecfg, 0, sizeof(xecfg));
memset(&state, 0, sizeof(state));
state.nmask = nmask;
@@ -231,7 +228,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
state.n = n;
xdi_diff_outf(&parent_file, result_file, consume_line, &state,
- &xpp, &xecfg, &ecb);
+ &xecfg, &ecb, 0);
free(parent_file.ptr);
/* Assign line numbers for this parent.
diff --git a/diff.c b/diff.c
index 29e608b..d95a928 100644
--- a/diff.c
+++ b/diff.c
@@ -698,7 +698,6 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
/* this executes the word diff on the accumulated buffers */
static void diff_words_show(struct diff_words_data *diff_words)
{
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
mmfile_t minus, plus;
@@ -714,15 +713,13 @@ static void diff_words_show(struct diff_words_data *diff_words)
diff_words->current_plus = diff_words->plus.text.ptr;
- memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
diff_words_fill(&diff_words->minus, &minus, diff_words->word_regex);
diff_words_fill(&diff_words->plus, &plus, diff_words->word_regex);
- xpp.flags = 0;
/* as only the hunk header will be parsed, we need a 0-context */
xecfg.ctxlen = 0;
xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
- &xpp, &xecfg, &ecb);
+ &xecfg, &ecb, 0);
free(minus.ptr);
free(plus.ptr);
if (diff_words->current_plus != diff_words->plus.text.ptr +
@@ -1703,7 +1700,6 @@ static void builtin_diff(const char *name_a,
else {
/* Crazy xdl interfaces.. */
const char *diffopts = getenv("GIT_DIFF_OPTS");
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
struct emit_callback ecbdata;
@@ -1733,7 +1729,6 @@ static void builtin_diff(const char *name_a,
if (!pe)
pe = diff_funcname_pattern(two);
- memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
memset(&ecbdata, 0, sizeof(ecbdata));
ecbdata.label_path = lbl;
@@ -1744,7 +1739,6 @@ static void builtin_diff(const char *name_a,
check_blank_at_eof(&mf1, &mf2, &ecbdata);
ecbdata.file = o->file;
ecbdata.header = header.len ? &header : NULL;
- xpp.flags = o->xdl_opts;
xecfg.ctxlen = o->context;
xecfg.interhunkctxlen = o->interhunkcontext;
xecfg.flags = XDL_EMIT_FUNCNAMES;
@@ -1777,7 +1771,7 @@ static void builtin_diff(const char *name_a,
}
}
xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,
- &xpp, &xecfg, &ecb);
+ &xecfg, &ecb, o->xdl_opts);
if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
free_diff_words_data(&ecbdata);
if (textconv_one)
@@ -1828,15 +1822,12 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
data->deleted = mf1.size;
} else {
/* Crazy xdl interfaces.. */
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
- memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
- xpp.flags = o->xdl_opts;
xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
- &xpp, &xecfg, &ecb);
+ &xecfg, &ecb, o->xdl_opts);
}
free_and_return:
@@ -1876,16 +1867,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
goto free_and_return;
else {
/* Crazy xdl interfaces.. */
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
- memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 1; /* at least one context line */
- xpp.flags = 0;
xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
- &xpp, &xecfg, &ecb);
+ &xecfg, &ecb, 0);
if (data.ws_rule & WS_BLANK_AT_EOF) {
struct emit_callback ecbdata;
@@ -3378,14 +3366,12 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
data.ctx = &ctx;
for (i = 0; i < q->nr; i++) {
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
mmfile_t mf1, mf2;
struct diff_filepair *p = q->queue[i];
int len1, len2;
- memset(&xpp, 0, sizeof(xpp));
memset(&xecfg, 0, sizeof(xecfg));
if (p->status == 0)
return error("internal diff status error");
@@ -3438,11 +3424,10 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)
len2, p->two->path);
git_SHA1_Update(&ctx, buffer, len1);
- xpp.flags = 0;
xecfg.ctxlen = 3;
xecfg.flags = XDL_EMIT_FUNCNAMES;
xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
- &xpp, &xecfg, &ecb);
+ &xecfg, &ecb, 0);
}
git_SHA1_Final(sha1, &ctx);
diff --git a/merge-file.c b/merge-file.c
index db4d0d5..6521561 100644
--- a/merge-file.c
+++ b/merge-file.c
@@ -61,12 +61,9 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
{
unsigned long size = f1->size < f2->size ? f1->size : f2->size;
void *ptr = xmalloc(size);
- xpparam_t xpp;
xdemitconf_t xecfg;
xdemitcb_t ecb;
- memset(&xpp, 0, sizeof(xpp));
- xpp.flags = 0;
memset(&xecfg, 0, sizeof(xecfg));
xecfg.ctxlen = 3;
xecfg.flags = XDL_EMIT_COMMON;
@@ -76,7 +73,7 @@ static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
res->size = 0;
ecb.priv = res;
- return xdi_diff(f1, f2, &xpp, &xecfg, &ecb);
+ return xdi_diff(f1, f2, &xecfg, &ecb, 0);
}
void *merge_file(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index ca5e3fb..6457a5b 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -126,20 +126,24 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
b->size -= trimmed - recovered;
}
-int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *xecb)
+int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xdemitconf_t const *xecfg,
+ xdemitcb_t *xecb, int xpp_flags)
{
+ xpparam_t xpp;
mmfile_t a = *mf1;
mmfile_t b = *mf2;
+ memset(&xpp, 0, sizeof(xpp));
+ xpp.flags = xpp_flags;
+
trim_common_tail(&a, &b, xecfg->ctxlen);
- return xdl_diff(&a, &b, xpp, xecfg, xecb);
+ return xdl_diff(&a, &b, &xpp, xecfg, xecb);
}
int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
xdiff_emit_consume_fn fn, void *consume_callback_data,
- xpparam_t const *xpp,
- xdemitconf_t const *xecfg, xdemitcb_t *xecb)
+ xdemitconf_t const *xecfg, xdemitcb_t *xecb, int xpp_flags)
{
int ret;
struct xdiff_emit_state state;
@@ -150,7 +154,7 @@ int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
xecb->outf = xdiff_outf;
xecb->priv = &state;
strbuf_init(&state.remainder, 0);
- ret = xdi_diff(mf1, mf2, xpp, xecfg, xecb);
+ ret = xdi_diff(mf1, mf2, xecfg, xecb, xpp_flags);
strbuf_release(&state.remainder);
return ret;
}
@@ -185,7 +189,7 @@ static int process_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
int xdi_diff_hunks(mmfile_t *mf1, mmfile_t *mf2,
xdiff_emit_hunk_consume_fn fn, void *consume_callback_data,
- xpparam_t const *xpp, xdemitconf_t *xecfg)
+ xdemitconf_t *xecfg, int xpp_flags)
{
struct xdiff_emit_hunk_state state;
xdemitcb_t ecb;
@@ -196,7 +200,7 @@ int xdi_diff_hunks(mmfile_t *mf1, mmfile_t *mf2,
state.consume_callback_data = consume_callback_data;
xecfg->emit_func = (void (*)())process_diff;
ecb.priv = &state;
- return xdi_diff(mf1, mf2, xpp, xecfg, &ecb);
+ return xdi_diff(mf1, mf2, xecfg, &ecb, xpp_flags);
}
int read_mmfile(mmfile_t *ptr, const char *filename)
diff --git a/xdiff-interface.h b/xdiff-interface.h
index abba70c..05adeb3 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -6,14 +6,14 @@
typedef void (*xdiff_emit_consume_fn)(void *, char *, unsigned long);
typedef void (*xdiff_emit_hunk_consume_fn)(void *, long, long, long);
-int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);
+int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xdemitconf_t const *xecfg,
+ xdemitcb_t *ecb, int xpp_flags);
int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
xdiff_emit_consume_fn fn, void *consume_callback_data,
- xpparam_t const *xpp,
- xdemitconf_t const *xecfg, xdemitcb_t *xecb);
+ xdemitconf_t const *xecfg, xdemitcb_t *xecb, int xpp_flags);
int xdi_diff_hunks(mmfile_t *mf1, mmfile_t *mf2,
xdiff_emit_hunk_consume_fn fn, void *consume_callback_data,
- xpparam_t const *xpp, xdemitconf_t *xecfg);
+ xdemitconf_t *xecfg, int xpp_flags);
int parse_hunk_header(char *line, int len,
int *ob, int *on,
int *nb, int *nn);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html