{"thread":{"id":"49632","subject":"[RFC/PATCH 0/3] refactor MIN macro","startedAt":"2018-10-22T05:36:24Z","lastAt":"2018-10-22T06:41:42Z","messageCount":8,"participants":["Carlo Marcelo Arenas Belón","Junio C Hamano","Carlo Arenas"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"361134","messageId":"20181022053605.81048-1-carenas@gmail.com","threadId":"49632","inReplyTo":null,"subject":"[RFC/PATCH 0/3] refactor MIN macro","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-22T05:36:02Z","receivedAt":"2018-10-22T05:36:24Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"otherwise will warn on platforms where it is already defined (macOS)\n\nconvert the internal version from xdiff to a common one from\nthe compatibilty header additionally\n\n\n"},{"id":"361135","messageId":"20181022053605.81048-2-carenas@gmail.com","threadId":"49632","inReplyTo":"20181022053605.81048-1-carenas@gmail.com","subject":"[PATCH 1/3] sha256: avoid redefinition for MIN","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-22T05:36:03Z","receivedAt":"2018-10-22T05:36:27Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"it is already defined whenever \"sys/param.h\" is available\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n sha256/block/sha256.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/sha256/block/sha256.c b/sha256/block/sha256.c\nindex 18350c161a..0d4939cc2c 100644\n--- a/sha256/block/sha256.c\n+++ b/sha256/block/sha256.c\n@@ -130,7 +130,9 @@ static void blk_SHA256_Transform(blk_SHA256_CTX *ctx, const unsigned char *buf)\n \t}\n }\n \n+#ifndef MIN\n #define MIN(x, y) ((x) < (y) ? (x) : (y))\n+#endif\n void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len)\n {\n \tconst unsigned char *in = data;\n-- \n2.19.1\n\n"},{"id":"361136","messageId":"20181022053605.81048-3-carenas@gmail.com","threadId":"49632","inReplyTo":"20181022053605.81048-1-carenas@gmail.com","subject":"[PATCH 2/3] git-compat-util: define MIN through compat","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-22T05:36:04Z","receivedAt":"2018-10-22T05:36:27Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"this macro is commonly defined in system headers (usually )\nbut if it is not define it here so it can be used elsewhere\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n git-compat-util.h | 5 +++++\n sha256/block/sha256.c | 3 ---\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 5f2e90932f..7a0b48452b 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -218,6 +218,11 @@\n #else\n #include \n #endif\n+\n+#ifndef MIN\n+#define MIN(x, y) ((x) < (y) ? (x) : (y))\n+#endif\n+\n #ifdef NO_INTPTR_T\n /*\n * On I16LP32, ILP32 and LP64 \"long\" is the safe bet, however\ndiff --git a/sha256/block/sha256.c b/sha256/block/sha256.c\nindex 0d4939cc2c..5fba943c79 100644\n--- a/sha256/block/sha256.c\n+++ b/sha256/block/sha256.c\n@@ -130,9 +130,6 @@ static void blk_SHA256_Transform(blk_SHA256_CTX *ctx, const unsigned char *buf)\n \t}\n }\n \n-#ifndef MIN\n-#define MIN(x, y) ((x) < (y) ? (x) : (y))\n-#endif\n void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len)\n {\n \tconst unsigned char *in = data;\n-- \n2.19.1\n\n"},{"id":"361137","messageId":"20181022053605.81048-4-carenas@gmail.com","threadId":"49632","inReplyTo":"20181022053605.81048-1-carenas@gmail.com","subject":"[PATCH 3/3] xdiff: use compat's MIN instead","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-22T05:36:05Z","receivedAt":"2018-10-22T05:36:31Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Signed-off-by: Carlo Marcelo Arenas Belón \n---\n xdiff/xdiffi.c | 2 +-\n xdiff/xemit.c | 6 +++---\n xdiff/xhistogram.c | 6 +++---\n xdiff/xmacros.h | 4 +---\n xdiff/xprepare.c | 2 +-\n 5 files changed, 9 insertions(+), 11 deletions(-)\n\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 1f1f4a3c78..0754dc17ed 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -204,7 +204,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,\n \n \t\t\tfbest = fbest1 = -1;\n \t\t\tfor (d = fmax; d >= fmin; d -= 2) {\n-\t\t\t\ti1 = XDL_MIN(kvdf[d], lim1);\n+\t\t\t\ti1 = MIN(kvdf[d], lim1);\n \t\t\t\ti2 = i1 - d;\n \t\t\t\tif (lim2 < i2)\n \t\t\t\t\ti1 = lim2 + d, i2 = lim2;\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 7778dc2b19..43d455b404 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -217,8 +217,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n post_context_calculation:\n \t\tlctx = xecfg->ctxlen;\n-\t\tlctx = XDL_MIN(lctx, xe->xdf1.nrec - (xche->i1 + xche->chg1));\n-\t\tlctx = XDL_MIN(lctx, xe->xdf2.nrec - (xche->i2 + xche->chg2));\n+\t\tlctx = MIN(lctx, xe->xdf1.nrec - (xche->i1 + xche->chg1));\n+\t\tlctx = MIN(lctx, xe->xdf2.nrec - (xche->i2 + xche->chg2));\n \n \t\te1 = xche->i1 + xche->chg1 + lctx;\n \t\te2 = xche->i2 + xche->chg2 + lctx;\n@@ -242,7 +242,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \t\t\t * its new end.\n \t\t\t */\n \t\t\tif (xche->next) {\n-\t\t\t\tlong l = XDL_MIN(xche->next->i1,\n+\t\t\t\tlong l = MIN(xche->next->i1,\n \t\t\t\t\t\t xe->xdf1.nrec - 1);\n \t\t\t\tif (l - xecfg->ctxlen <= e1 ||\n \t\t\t\t get_func_line(xe, xecfg, NULL, l, e1) < 0) {\ndiff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c\nindex ec85f5992b..562cab6d14 100644\n--- a/xdiff/xhistogram.c\n+++ b/xdiff/xhistogram.c\n@@ -129,7 +129,7 @@ static int scanA(struct histindex *index, int line1, int count1)\n \t\t\t\tNEXT_PTR(index, ptr) = rec->ptr;\n \t\t\t\trec->ptr = ptr;\n \t\t\t\t/* cap rec->cnt at MAX_CNT */\n-\t\t\t\trec->cnt = XDL_MIN(MAX_CNT, rec->cnt + 1);\n+\t\t\t\trec->cnt = MIN(MAX_CNT, rec->cnt + 1);\n \t\t\t\tLINE_MAP(index, ptr) = rec;\n \t\t\t\tgoto continue_scan;\n \t\t\t}\n@@ -193,14 +193,14 @@ static int try_lcs(struct histindex *index, struct region *lcs, int b_ptr,\n \t\t\t\tas--;\n \t\t\t\tbs--;\n \t\t\t\tif (1 < rc)\n-\t\t\t\t\trc = XDL_MIN(rc, CNT(index, as));\n+\t\t\t\t\trc = MIN(rc, CNT(index, as));\n \t\t\t}\n \t\t\twhile (ae < LINE_END(1) && be < LINE_END(2)\n \t\t\t\t&& CMP(index, 1, ae + 1, 2, be + 1)) {\n \t\t\t\tae++;\n \t\t\t\tbe++;\n \t\t\t\tif (1 < rc)\n-\t\t\t\t\trc = XDL_MIN(rc, CNT(index, ae));\n+\t\t\t\t\trc = MIN(rc, CNT(index, ae));\n \t\t\t}\n \n \t\t\tif (b_next <= be)\ndiff --git a/xdiff/xmacros.h b/xdiff/xmacros.h\nindex 2809a28ca9..495b9dfac4 100644\n--- a/xdiff/xmacros.h\n+++ b/xdiff/xmacros.h\n@@ -23,10 +23,8 @@\n #if !defined(XMACROS_H)\n #define XMACROS_H\n \n+#include \"../git-compat-util.h\"\n \n-\n-\n-#define XDL_MIN(a, b) ((a) < (b) ? (a): (b))\n #define XDL_MAX(a, b) ((a) > (b) ? (a): (b))\n #define XDL_ABS(v) ((v) >= 0 ? (v): -(v))\n #define XDL_ISDIGIT(c) ((c) >= '0' && (c) <= '9')\ndiff --git a/xdiff/xprepare.c b/xdiff/xprepare.c\nindex abeb8fb84e..7436064498 100644\n--- a/xdiff/xprepare.c\n+++ b/xdiff/xprepare.c\n@@ -451,7 +451,7 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {\n \n \trecs1 = xdf1->recs;\n \trecs2 = xdf2->recs;\n-\tfor (i = 0, lim = XDL_MIN(xdf1->nrec, xdf2->nrec); i < lim;\n+\tfor (i = 0, lim = MIN(xdf1->nrec, xdf2->nrec); i < lim;\n \t i++, recs1++, recs2++)\n \t\tif ((*recs1)->ha != (*recs2)->ha)\n \t\t\tbreak;\n-- \n2.19.1\n\n"},{"id":"361140","messageId":"xmqqsh0yv7wg.fsf@gitster-ct.c.googlers.com","threadId":"49632","inReplyTo":"20181022053605.81048-4-carenas@gmail.com","subject":"Re: [PATCH 3/3] xdiff: use compat's MIN instead","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-22T05:59:27Z","receivedAt":"2018-10-22T05:59:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón writes:\n\n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n> xdiff/xdiffi.c | 2 +-\n> xdiff/xemit.c | 6 +++---\n> xdiff/xhistogram.c | 6 +++---\n> xdiff/xmacros.h | 4 +---\n> xdiff/xprepare.c | 2 +-\n> 5 files changed, 9 insertions(+), 11 deletions(-)\n\nAs this is a borrowed code, I'd rather not do this, especially do so\nonly for MIN so that people who update this need to be aware that\nthey can say MIN() but still have to say XDL_MAX().\n\n> -\n> -#define XDL_MIN(a, b) ((a) < (b) ? (a): (b))\n> #define XDL_MAX(a, b) ((a) > (b) ? (a): (b))\n\n"},{"id":"361141","messageId":"xmqqo9bmv7ud.fsf@gitster-ct.c.googlers.com","threadId":"49632","inReplyTo":"20181022053605.81048-2-carenas@gmail.com","subject":"Re: [PATCH 1/3] sha256: avoid redefinition for MIN","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-22T06:00:42Z","receivedAt":"2018-10-22T06:00:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón writes:\n\n> it is already defined whenever \"sys/param.h\" is available\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n> sha256/block/sha256.c | 2 ++\n> 1 file changed, 2 insertions(+)\n\nIt is a no-brainer to say that this is obviously good. I'd rather\nsee this become a part of sha256 topic (perhaps squash it in to\neliminate the need to have it as a separate patch), though.\n\nThanks.\n\n>\n> diff --git a/sha256/block/sha256.c b/sha256/block/sha256.c\n> index 18350c161a..0d4939cc2c 100644\n> --- a/sha256/block/sha256.c\n> +++ b/sha256/block/sha256.c\n> @@ -130,7 +130,9 @@ static void blk_SHA256_Transform(blk_SHA256_CTX *ctx, const unsigned char *buf)\n> \t}\n> }\n> \n> +#ifndef MIN\n> #define MIN(x, y) ((x) < (y) ? (x) : (y))\n> +#endif\n> void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len)\n> {\n> \tconst unsigned char *in = data;\n"},{"id":"361142","messageId":"xmqqin1uv78g.fsf@gitster-ct.c.googlers.com","threadId":"49632","inReplyTo":"20181022053605.81048-3-carenas@gmail.com","subject":"Re: [PATCH 2/3] git-compat-util: define MIN through compat","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-22T06:13:51Z","receivedAt":"2018-10-22T06:13:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón writes:\n\n> this macro is commonly defined in system headers (usually )\n> but if it is not define it here so it can be used elsewhere\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón \n> ---\n\nI am between \"meh\" and \"moderately negative\" on this change.\n\n - Definition of MIN without matching MAX makes me worried about\n longer-term maintainability. If we were to add MIN() we should\n also add MAX() to match.\n\n However, \"git grep -e '#define MAX(' -e '#define MIN('\" tells me\n that we do not use these anywhere and Brian's use of a single\n MIN() is the only thing that makes this patch interesting. That\n is the primary reason why I am very close to \"meh\" on this.\n\n - We need to make sure that this is enough in \"git-compat-util.h\".\n Ideally, this should be after all \"#include\" of the system\n supplied header files, and before any use of MIN() macro\n ourselves. I am reasonably sure that the place this patch chose\n to insert the definition satisfies that criterion within the\n context of the today's code, but I am unsure if it is even worth\n having to worry about it in the first place, given how rarely if\n ever the macro is used. I think Brian's reroll of the sha256\n series even gets rid of the use of the macro, as it had only a\n single place that used the macro anyway.\n\nSo...\n\n> git-compat-util.h | 5 +++++\n> sha256/block/sha256.c | 3 ---\n> 2 files changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 5f2e90932f..7a0b48452b 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -218,6 +218,11 @@\n> #else\n> #include \n> #endif\n> +\n> +#ifndef MIN\n> +#define MIN(x, y) ((x) < (y) ? (x) : (y))\n> +#endif\n> +\n> #ifdef NO_INTPTR_T\n> /*\n> * On I16LP32, ILP32 and LP64 \"long\" is the safe bet, however\n> diff --git a/sha256/block/sha256.c b/sha256/block/sha256.c\n> index 0d4939cc2c..5fba943c79 100644\n> --- a/sha256/block/sha256.c\n> +++ b/sha256/block/sha256.c\n> @@ -130,9 +130,6 @@ static void blk_SHA256_Transform(blk_SHA256_CTX *ctx, const unsigned char *buf)\n> \t}\n> }\n> \n> -#ifndef MIN\n> -#define MIN(x, y) ((x) < (y) ? (x) : (y))\n> -#endif\n> void blk_SHA256_Update(blk_SHA256_CTX *ctx, const void *data, size_t len)\n> {\n> \tconst unsigned char *in = data;\n"},{"id":"361143","messageId":"CAPUEspibPrTk-nG4iiKZD0Sx_hxqD0hTEhqMx0BdG_tPCLtEQw@mail.gmail.com","threadId":"49632","inReplyTo":"xmqqin1uv78g.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] git-compat-util: define MIN through compat","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2018-10-22T06:41:27Z","receivedAt":"2018-10-22T06:41:42Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"I agree with you; dropped\n"}]}