{"thread":{"id":"53626","subject":"[PATCH v1] builtin/checkout.c: replace bit-shift values for macros in call to check_stages","startedAt":"2020-06-07T05:49:22Z","lastAt":"2020-06-10T05:15:23Z","messageCount":4,"participants":["Edmundo Carmona Antoranz","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"399236","messageId":"20200607054849.1014185-1-eantoranz@gmail.com","threadId":"53626","inReplyTo":null,"subject":"[PATCH v1] builtin/checkout.c: replace bit-shift values for macros in call to check_stages","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2020-06-07T05:48:49Z","receivedAt":"2020-06-07T05:49:22Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"Currently a call to check_stages is using bit-shift values to define what will be used\nas the first argument. If the reader doesn't know what the values are from heart, they\nhave to go dig in cache.h what each value is.\n\nThis patch is replacing those values for the macros from cache.h so that it's\nclear at first sight what they are (CE_ENTRY_REMOVED, CE_ENTRY_ADDED)\n\nSigned-off-by: Edmundo Carmona Antoranz \n---\n builtin/checkout.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex ffa776c6e1..3a644b31f6 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -397,7 +397,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t\t\t} else if (opts->writeout_stage) {\n \t\t\t\terrs |= check_stage(opts->writeout_stage, ce, pos, opts->overlay_mode);\n \t\t\t} else if (opts->merge) {\n-\t\t\t\terrs |= check_stages((1<<2) | (1<<3), ce, pos);\n+\t\t\t\terrs |= check_stages(CE_ENTRY_REMOVED | CE_ENTRY_ADDED, ce, pos);\n \t\t\t} else {\n \t\t\t\terrs = 1;\n \t\t\t\terror(_(\"path '%s' is unmerged\"), ce->name);\n-- \n2.26.2\n\n"},{"id":"399237","messageId":"CAOc6etZGw5SUj_ijWZtDjN44i7WLMKrUKALCpmfPAPxZK1c-tw@mail.gmail.com","threadId":"53626","inReplyTo":"20200607054849.1014185-1-eantoranz@gmail.com","subject":"Re: [PATCH v1] builtin/checkout.c: replace bit-shift values for macros in call to check_stages","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2020-06-07T05:54:06Z","receivedAt":"2020-06-07T05:54:32Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Sat, Jun 6, 2020 at 11:49 PM Edmundo Carmona Antoranz\n wrote:\n>\n> Currently a call to check_stages is using bit-shift values to define what will be used\n> as the first argument. If the reader doesn't know what the values are from heart, they\n> have to go dig in cache.h what each value is.\n>\n> This patch is replacing those values for the macros from cache.h so that it's\n> clear at first sight what they are (CE_ENTRY_REMOVED, CE_ENTRY_ADDED)\n\n\nThe macros are defined since at least v2.1.0 and I am able to apply\nthe patch on top of it. Should\nI send the patch from that version? I am sending on top of v2.22.4\ncause it's the oldest version I\nwas able to compile successfully.\n"},{"id":"399364","messageId":"xmqqwo4ggfb5.fsf@gitster.c.googlers.com","threadId":"53626","inReplyTo":"20200607054849.1014185-1-eantoranz@gmail.com","subject":"Re: [PATCH v1] builtin/checkout.c: replace bit-shift values for macros in call to check_stages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-09T20:22:22Z","receivedAt":"2020-06-09T20:22:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz writes:\n\n> Currently a call to check_stages is using bit-shift values to\n> define what will be used as the first argument. If the reader\n> doesn't know what the values are from heart, they have to go dig\n> in cache.h what each value is.\n>\n> This patch is replacing those values for the macros from cache.h so that it's\n> clear at first sight what they are (CE_ENTRY_REMOVED, CE_ENTRY_ADDED)\n\nThe patch does not make sense. The hardcoded numbers you touched\nhave nothing to do with these macros.\n\nThe first argument to check_stages() is a bitmask for the stage\nnumbers in the index (which can span from 1 to 3), where 1 means\n\"common ancestor\", 2 means \"ours\" and 3 means \"theirs\". The caller\ngives a position 'pos' in the in-core index and asks the function\nthis question: \"starting at position 'pos', there must be cache\nentries with the same path as 'ce'---are these enries cover all the\nstages specified by this mask?\" This particular caller wants to\nmake sure that there are both \"ours\" and \"theirs\" entries for the\npath.\n\nThe thing is, CE_ENTRY_REMOVED may also happen to have the value of\n4, but that has nothing to do with (1<<2) used in the argument of\nthis. It was not chosen to be the value of 1 shifted by the stage\nnumber for \"ours\". Same thing for CE_ENTRY_ADDED and (1<<3). Even\nwhen these macros are renumbered, the argument to check_stages()\nmust stay the values they are written in the original code\n(i.e. (1<<2)|(1<<3) == 12).\n\n> Signed-off-by: Edmundo Carmona Antoranz \n> ---\n> builtin/checkout.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index ffa776c6e1..3a644b31f6 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -397,7 +397,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n> \t\t\t} else if (opts->writeout_stage) {\n> \t\t\t\terrs |= check_stage(opts->writeout_stage, ce, pos, opts->overlay_mode);\n> \t\t\t} else if (opts->merge) {\n> -\t\t\t\terrs |= check_stages((1<<2) | (1<<3), ce, pos);\n> +\t\t\t\terrs |= check_stages(CE_ENTRY_REMOVED | CE_ENTRY_ADDED, ce, pos);\n> \t\t\t} else {\n> \t\t\t\terrs = 1;\n> \t\t\t\terror(_(\"path '%s' is unmerged\"), ce->name);\n"},{"id":"399387","messageId":"CAOc6etaRe8a=njHxwdTsqFVP0Myb0MVrBMo_+2hcChJ0RJasaA@mail.gmail.com","threadId":"53626","inReplyTo":"xmqqwo4ggfb5.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v1] builtin/checkout.c: replace bit-shift values for macros in call to check_stages","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2020-06-10T05:15:08Z","receivedAt":"2020-06-10T05:15:23Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Tue, Jun 9, 2020 at 2:22 PM Junio C Hamano wrote:\n> The patch does not make sense. The hardcoded numbers you touched\n> have nothing to do with these macros.\n>\n\nTotally misguided myself. Sorry for the confusion.\n"}]}