{"thread":{"id":"41994","subject":"(unknown)","startedAt":"2016-04-11T19:04:23Z","lastAt":"2016-04-12T04:33:07Z","messageCount":4,"participants":["miwilliams@google.com","Jeff King","Matthieu Moy","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"283149","messageId":"001a11492f107354a305303a369a@google.com","threadId":"41994","inReplyTo":null,"subject":"(unknown)","fromName":"","fromEmail":"miwilliams@google.com","sentAt":"2016-04-11T19:04:23Z","receivedAt":"2016-04-11T19:04:23Z","isPatch":false,"sender":{"key":"miwilliams@google.com","avatar":null},"body":" From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001\nFrom: Mike Williams \nDate: Mon, 11 Apr 2016 14:18:39 -0400\nSubject: [PATCH 1/1] wt-status: Remove '!!' from \nwt_status_collect_changed_cb\n\nThe wt_status_collect_changed_cb function uses an extraneous double \nnegation (!!)\nwhen determining whether or not a submodule has new commits.\n\nSigned-off-by: Mike Williams \n---\n wt-status.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex ef74864..b955179 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct \ndiff_queue_struct *q,\n \t\t\td->worktree_status = p->status;\n \t\td->dirty_submodule = p->two->dirty_submodule;\n \t\tif (S_ISGITLINK(p->two->mode))\n-\t\t\td->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);\n+\t\t\td->new_submodule_commits = hashcmp(p->one->sha1, p->two->sha1);\n \t}\n }\n\n-- \n2.8.0\n"},{"id":"283150","messageId":"20160411191311.GA9057@sigill.intra.peff.net","threadId":"41994","inReplyTo":"001a11492f107354a305303a369a@google.com","subject":"Re: your mail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-11T19:13:12Z","receivedAt":"2016-04-11T19:13:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 11, 2016 at 07:04:23PM +0000, miwilliams@google.com wrote:\n\n> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001\n> From: Mike Williams \n> Date: Mon, 11 Apr 2016 14:18:39 -0400\n> Subject: [PATCH 1/1] wt-status: Remove '!!' from\n> wt_status_collect_changed_cb\n\nThese bits (minus the initial \"From ...\" line) should go into your\nactual email headers. As it is, your email has no subject line.\n\n> The wt_status_collect_changed_cb function uses an extraneous double\n> negation (!!) when determining whether or not a submodule has new\n> commits.\n> [...]\n> -\t\t\td->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);\n> +\t\t\td->new_submodule_commits = hashcmp(p->one->sha1, p->two->sha1);\n\nIt's not extraneous. hashcmp() returns 0 for equality, but an arbitrary\npositive or negative value depending on how the two arguments differ.\nThe assigned \"new_submodule_commits\" is a bitfield of size 1. So the\n\"!!\" is normalizing the value into \"0\" or \"1\".\n\n-Peff\n"},{"id":"283151","messageId":"vpq37qsosyf.fsf@anie.imag.fr","threadId":"41994","inReplyTo":"001a11492f107354a305303a369a@google.com","subject":"Re: none","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-04-11T19:18:00Z","receivedAt":"2016-04-11T19:18:00Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"miwilliams@google.com writes:\n\n> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001\n> From: Mike Williams \n> Date: Mon, 11 Apr 2016 14:18:39 -0400\n> Subject: [PATCH 1/1] wt-status: Remove '!!' from\n> wt_status_collect_changed_cb\n>\n> The wt_status_collect_changed_cb function uses an extraneous double\n> negation (!!)\n> when determining whether or not a submodule has new commits.\n\nIt's not just a double negation, it's a way to ensure that the value is\n0 or 1 (it's a relatively common idiom in C at least in Git's codebase).\n\nnew_submodule_commits is a 1-bit bitfield, and you don't want to assign\nanything other than 1 or 0 (or you'll get modulo 2^n semantics, with\nn==1).\n\nSo the old code is correct and your patch would introduce a bug.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"283166","messageId":"CAGZ79kYD7o3O_-zSGD24_DUy9hDc9=u6pPYXOOWp+-i-PW7vjQ@mail.gmail.com","threadId":"41994","inReplyTo":"001a11492f107354a305303a369a@google.com","subject":"Re:","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-12T04:33:07Z","receivedAt":"2016-04-12T04:33:07Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Apr 11, 2016 at 12:04 PM, wrote:\n> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001\n> From: Mike Williams \n> Date: Mon, 11 Apr 2016 14:18:39 -0400\n> Subject: [PATCH 1/1] wt-status: Remove '!!' from\n> wt_status_collect_changed_cb\n>\n> The wt_status_collect_changed_cb function uses an extraneous double negation\n> (!!)\n\nHow is an !! errornous?\n\nIt serves the purpose to map an integer value(-1,0,1,2,3,4)\nto a boolean (0,1, or a real bit in a bit field).\n\n> when determining whether or not a submodule has new commits.\n>\n> Signed-off-by: Mike Williams \n> ---\n> wt-status.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/wt-status.c b/wt-status.c\n> index ef74864..b955179 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct\n> diff_queue_struct *q,\n> d->worktree_status = p->status;\n> d->dirty_submodule = p->two->dirty_submodule;\n> if (S_ISGITLINK(p->two->mode))\n> - d->new_submodule_commits = !!hashcmp(p->one->sha1,\n> p->two->sha1);\n> + d->new_submodule_commits = hashcmp(p->one->sha1,\n> p->two->sha1);\n> }\n> }\n>\n> --\n> 2.8.0\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"}]}