{"thread":{"id":"23698","subject":"[PATCH][CLEANUP] remove ecb parameter from xdi_diff_outf()","startedAt":"2010-05-04T20:41:34Z","lastAt":"2010-05-04T20:58:53Z","messageCount":2,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"140934","messageId":"4BE0867E.3060906@lsrfire.ath.cx","threadId":"23698","inReplyTo":null,"subject":"[PATCH][CLEANUP] remove ecb parameter from xdi_diff_outf()","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-05-04T20:41:34Z","receivedAt":"2010-05-04T20:41:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"xdi_diff_outf() overrides the structure members of its last parameter,\nignoring any value that callers pass in.  It's no surprise then that all\ncallers pass a pointer to an uninitialized structure.  They also don't\nread it after the call, so the parameter is neither used for input nor\nfor output.   Turn it into a local variable of xdi_diff_outf().\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n combine-diff.c    |    3 +--\n diff.c            |   15 +++++----------\n xdiff-interface.c |   11 ++++++-----\n xdiff-interface.h |    3 +--\n 4 files changed, 13 insertions(+), 19 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 3480dae..7557136 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -211,7 +211,6 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \txpparam_t xpp;\n \txdemitconf_t xecfg;\n \tmmfile_t parent_file;\n-\txdemitcb_t ecb;\n \tstruct combine_diff_state state;\n \tunsigned long sz;\n \n@@ -231,7 +230,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,\n \tstate.n = n;\n \n \txdi_diff_outf(&parent_file, result_file, consume_line, &state,\n-\t\t      &xpp, &xecfg, &ecb);\n+\t\t      &xpp, &xecfg);\n \tfree(parent_file.ptr);\n \n \t/* Assign line numbers for this parent.\ndiff --git a/diff.c b/diff.c\nindex d0ecbc3..e40c127 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -700,7 +700,6 @@ static void diff_words_show(struct diff_words_data *diff_words)\n {\n \txpparam_t xpp;\n \txdemitconf_t xecfg;\n-\txdemitcb_t ecb;\n \tmmfile_t minus, plus;\n \n \t/* special case: only removal */\n@@ -722,7 +721,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \t/* as only the hunk header will be parsed, we need a 0-context */\n \txecfg.ctxlen = 0;\n \txdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,\n-\t\t      &xpp, &xecfg, &ecb);\n+\t\t      &xpp, &xecfg);\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n \tif (diff_words->current_plus != diff_words->plus.text.ptr +\n@@ -1708,7 +1707,6 @@ static void builtin_diff(const char *name_a,\n \t\tconst char *diffopts = getenv(\"GIT_DIFF_OPTS\");\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \t\tstruct emit_callback ecbdata;\n \t\tconst struct userdiff_funcname *pe;\n \n@@ -1780,7 +1778,7 @@ static void builtin_diff(const char *name_a,\n \t\t\t}\n \t\t}\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \t\tif (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))\n \t\t\tfree_diff_words_data(&ecbdata);\n \t\tif (textconv_one)\n@@ -1833,13 +1831,12 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\t/* Crazy xdl interfaces.. */\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \n \t\tmemset(&xpp, 0, sizeof(xpp));\n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \t}\n \n  free_and_return:\n@@ -1881,14 +1878,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\t/* Crazy xdl interfaces.. */\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \n \t\tmemset(&xpp, 0, sizeof(xpp));\n \t\tmemset(&xecfg, 0, sizeof(xecfg));\n \t\txecfg.ctxlen = 1; /* at least one context line */\n \t\txpp.flags = XDF_NEED_MINIMAL;\n \t\txdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \n \t\tif (data.ws_rule & WS_BLANK_AT_EOF) {\n \t\t\tstruct emit_callback ecbdata;\n@@ -3383,7 +3379,6 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \tfor (i = 0; i < q->nr; i++) {\n \t\txpparam_t xpp;\n \t\txdemitconf_t xecfg;\n-\t\txdemitcb_t ecb;\n \t\tmmfile_t mf1, mf2;\n \t\tstruct diff_filepair *p = q->queue[i];\n \t\tint len1, len2;\n@@ -3445,7 +3440,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1)\n \t\txecfg.ctxlen = 3;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n \t\txdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,\n-\t\t\t      &xpp, &xecfg, &ecb);\n+\t\t\t      &xpp, &xecfg);\n \t}\n \n \tgit_SHA1_Final(sha1, &ctx);\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex ca5e3fb..cd2285d 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -138,19 +138,20 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co\n \n int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,\n \t\t  xdiff_emit_consume_fn fn, void *consume_callback_data,\n-\t\t  xpparam_t const *xpp,\n-\t\t  xdemitconf_t const *xecfg, xdemitcb_t *xecb)\n+\t\t  xpparam_t const *xpp, xdemitconf_t const *xecfg)\n {\n \tint ret;\n \tstruct xdiff_emit_state state;\n+\txdemitcb_t ecb;\n \n \tmemset(&state, 0, sizeof(state));\n \tstate.consume = fn;\n \tstate.consume_callback_data = consume_callback_data;\n-\txecb->outf = xdiff_outf;\n-\txecb->priv = &state;\n+\tmemset(&ecb, 0, sizeof(ecb));\n+\tecb.outf = xdiff_outf;\n+\tecb.priv = &state;\n \tstrbuf_init(&state.remainder, 0);\n-\tret = xdi_diff(mf1, mf2, xpp, xecfg, xecb);\n+\tret = xdi_diff(mf1, mf2, xpp, xecfg, &ecb);\n \tstrbuf_release(&state.remainder);\n \treturn ret;\n }\ndiff --git a/xdiff-interface.h b/xdiff-interface.h\nindex abba70c..49d1116 100644\n--- a/xdiff-interface.h\n+++ b/xdiff-interface.h\n@@ -9,8 +9,7 @@ typedef void (*xdiff_emit_hunk_consume_fn)(void *, long, long, long);\n int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);\n int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,\n \t\t  xdiff_emit_consume_fn fn, void *consume_callback_data,\n-\t\t  xpparam_t const *xpp,\n-\t\t  xdemitconf_t const *xecfg, xdemitcb_t *xecb);\n+\t\t  xpparam_t const *xpp, xdemitconf_t const *xecfg);\n int xdi_diff_hunks(mmfile_t *mf1, mmfile_t *mf2,\n \t\t   xdiff_emit_hunk_consume_fn fn, void *consume_callback_data,\n \t\t   xpparam_t const *xpp, xdemitconf_t *xecfg);\n-- \n1.7.1\n"},{"id":"140935","messageId":"20100504205853.GA10716@coredump.intra.peff.net","threadId":"23698","inReplyTo":"4BE0867E.3060906@lsrfire.ath.cx","subject":"Re: [PATCH][CLEANUP] remove ecb parameter from xdi_diff_outf()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-05-04T20:58:53Z","receivedAt":"2010-05-04T20:58:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 04, 2010 at 10:41:34PM +0200, René Scharfe wrote:\n\n> xdi_diff_outf() overrides the structure members of its last parameter,\n> ignoring any value that callers pass in.  It's no surprise then that all\n> callers pass a pointer to an uninitialized structure.  They also don't\n> read it after the call, so the parameter is neither used for input nor\n> for output.   Turn it into a local variable of xdi_diff_outf().\n\nHeh:\n\n  http://article.gmane.org/gmane.comp.version-control.git/102861\n\nSo,\n\nAcked-by: Jeff King <peff@peff.net>\n\n-Peff\n"}]}