{"thread":{"id":"60983","subject":"[GSoC][PATCH 0/1] Use unsigned integral type for collection of bits.","startedAt":"2024-02-24T11:26:54Z","lastAt":"2024-02-29T19:52:15Z","messageCount":7,"participants":["Eugenio Gigante","Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"489276","messageId":"20240224112638.72257-1-giganteeugenio2@gmail.com","threadId":"60983","inReplyTo":null,"subject":"[GSoC][PATCH 0/1] Use unsigned integral type for collection of bits.","fromName":"Eugenio Gigante","fromEmail":"giganteeugenio2@gmail.com","sentAt":"2024-02-24T11:26:37Z","receivedAt":"2024-02-24T11:26:54Z","isPatch":true,"sender":{"key":"giganteeugenio2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116371051?v=4"},"body":"One of the suggested microprojects for the GSoC is to pick a field\nof signed integral type that is used as as collection of bits, and\nchange its type to unsigned in case the code does not take advantage\nof its MSb.\nThis patch finds an example in 'builtin/add.c'.\n\n\nEugenio Gigante (1):\n add: use unsigned type for collection of bits\n\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\n\nbase-commit: 2a540e432fe5dff3cfa9d3bf7ca56db2ad12ebb9\n-- \n2.43.0\n\n"},{"id":"489277","messageId":"20240224112638.72257-2-giganteeugenio2@gmail.com","threadId":"60983","inReplyTo":"20240224112638.72257-1-giganteeugenio2@gmail.com","subject":"[GSoC][PATCH 1/1] add: use unsigned type for collection of bits","fromName":"Eugenio Gigante","fromEmail":"giganteeugenio2@gmail.com","sentAt":"2024-02-24T11:26:38Z","receivedAt":"2024-02-24T11:27:57Z","isPatch":true,"sender":{"key":"giganteeugenio2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116371051?v=4"},"body":"The function 'refresh' in 'builtin/add.c' declares 'flags' as signed,\nwhile the function 'refresh_index' defined in 'read-cache-ll.h' expects an unsigned value.\nSince in this case 'flags' represents a bag of bits, whose MSB is not used in special ways,\nthis commit changes the type of 'flags' to unsigned.\n\nSigned-off-by: Eugenio Gigante \n---\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ada7719561..393c10cbcf 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -115,7 +115,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)\n \tint i, ret = 0;\n \tchar *skip_worktree_seen = NULL;\n \tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n-\tint flags = REFRESH_IGNORE_SKIP_WORKTREE |\n+\tunsigned int flags = REFRESH_IGNORE_SKIP_WORKTREE |\n \t\t (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);\n \n \tseen = xcalloc(pathspec->nr, 1);\n-- \n2.43.0\n\n"},{"id":"489360","messageId":"CAP8UFD3qR8E0gvUQtzzkLPWv4Db45kFS4pEqHKQr5siciVJ-zQ@mail.gmail.com","threadId":"60983","inReplyTo":"20240224112638.72257-2-giganteeugenio2@gmail.com","subject":"Re: [GSoC][PATCH 1/1] add: use unsigned type for collection of bits","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-02-26T09:59:05Z","receivedAt":"2024-02-26T09:59:19Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Feb 24, 2024 at 12:28 PM Eugenio Gigante\n wrote:\n>\n> The function 'refresh' in 'builtin/add.c' declares 'flags' as signed,\n> while the function 'refresh_index' defined in 'read-cache-ll.h' expects an unsigned value.\n\nIt's not clear from the patch that refresh() passes 'flags' as an\nargument to refresh_index(), so it might help reviewers a bit if you\ncould tell that.\n\n> Since in this case 'flags' represents a bag of bits, whose MSB is not used in special ways,\n> this commit changes the type of 'flags' to unsigned.\n\nWe prefer to use \"let's change this and that\" or just \"change this and\nthat\" rather than \"this commit changes this and that\", see\nhttps://git-scm.com/docs/SubmittingPatches/#imperative-mood.\n\nIt might help if you could add a bit more explanation about why it's a\ngood thing to use an unsigned variable instead of a signed one. For\nexample you could say that it documents that we are not doing anything\nfunny with the MSB.\n\n> Signed-off-by: Eugenio Gigante \n> ---\n> builtin/add.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nThe patch looks correct, thanks!\n"},{"id":"489441","messageId":"xmqqsf1ekf34.fsf@gitster.g","threadId":"60983","inReplyTo":"CAP8UFD3qR8E0gvUQtzzkLPWv4Db45kFS4pEqHKQr5siciVJ-zQ@mail.gmail.com","subject":"Re: [GSoC][PATCH 1/1] add: use unsigned type for collection of bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-26T22:58:39Z","receivedAt":"2024-02-26T22:58:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> On Sat, Feb 24, 2024 at 12:28 PM Eugenio Gigante\n> wrote:\n>>\n>> The function 'refresh' in 'builtin/add.c' declares 'flags' as\n>> signed, while the function 'refresh_index' defined in\n>> 'read-cache-ll.h' expects an unsigned value.\n>\n> It's not clear from the patch that refresh() passes 'flags' as an\n> argument to refresh_index(), so it might help reviewers a bit if you\n> could tell that.\n\nPerhaps.\n\n>> Since in this case 'flags' represents a bag of bits, whose MSB is\n>> not used in special ways, this commit changes the type of 'flags'\n>> to unsigned.\n>\n> We prefer to use \"let's change this and that\" or just \"change this and\n> that\" rather than \"this commit changes this and that\", see\n> https://git-scm.com/docs/SubmittingPatches/#imperative-mood.\n\nVery true.\n\n> It might help if you could add a bit more explanation about why it's a\n> good thing to use an unsigned variable instead of a signed one. For\n> example you could say that it documents that we are not doing anything\n> funny with the MSB.\n\nBut doesn't the proposed log message already say so?\n\nIn any case, it would very much help to fold long lines and have a\nblank line in between paragraphs to make the log message more\nreadable.\n\nThanks, both.\n\n\n\n>\n>> Signed-off-by: Eugenio Gigante \n>> ---\n>> builtin/add.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> The patch looks correct, thanks!\n"},{"id":"489683","messageId":"20240229194444.8499-1-giganteeugenio2@gmail.com","threadId":"60983","inReplyTo":"xmqqsf1ekf34.fsf@gitster.g","subject":"[GSoC][PATCH v2 0/1] Use unsigned integral type for collection of bits.","fromName":"Eugenio Gigante","fromEmail":"giganteeugenio2@gmail.com","sentAt":"2024-02-29T19:44:43Z","receivedAt":"2024-02-29T19:44:51Z","isPatch":true,"sender":{"key":"giganteeugenio2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116371051?v=4"},"body":"Improve the commit log message\nbased on reviewers feedback.\n\nEugenio Gigante (1):\n add: use unsigned type for collection of bits\n\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\n\nbase-commit: 2a540e432fe5dff3cfa9d3bf7ca56db2ad12ebb9\n-- \n2.43.0\n\n"},{"id":"489684","messageId":"20240229194444.8499-2-giganteeugenio2@gmail.com","threadId":"60983","inReplyTo":"20240229194444.8499-1-giganteeugenio2@gmail.com","subject":"[PATCH v2 1/1] add: use unsigned type for collection of bits","fromName":"Eugenio Gigante","fromEmail":"giganteeugenio2@gmail.com","sentAt":"2024-02-29T19:44:44Z","receivedAt":"2024-02-29T19:44:57Z","isPatch":true,"sender":{"key":"giganteeugenio2@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116371051?v=4"},"body":"The 'refresh' function in 'builtin/add.c'\ndeclares 'flags' as signed, and passes it\nas an argument to the 'refresh_index' function,\nwhich though expects an unsigned value.\n\nSince in this case 'flags' represents\na bag of bits, whose MSB is not used\nin special ways, change the type\nof 'flags' to unsigned.\n\nSigned-off-by: Eugenio Gigante \n---\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ada7719561..393c10cbcf 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -115,7 +115,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)\n \tint i, ret = 0;\n \tchar *skip_worktree_seen = NULL;\n \tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n-\tint flags = REFRESH_IGNORE_SKIP_WORKTREE |\n+\tunsigned int flags = REFRESH_IGNORE_SKIP_WORKTREE |\n \t\t (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);\n \n \tseen = xcalloc(pathspec->nr, 1);\n-- \n2.43.0\n\n"},{"id":"489685","messageId":"xmqqttlrdp5f.fsf@gitster.g","threadId":"60983","inReplyTo":"20240229194444.8499-2-giganteeugenio2@gmail.com","subject":"Re: [PATCH v2 1/1] add: use unsigned type for collection of bits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-29T19:52:12Z","receivedAt":"2024-02-29T19:52:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eugenio Gigante writes:\n\n> The 'refresh' function in 'builtin/add.c'\n> declares 'flags' as signed, and passes it\n> as an argument to the 'refresh_index' function,\n> which though expects an unsigned value.\n\nOK. This has become much easier to read by dropping the mention of\nthe header file where the callee happens to be declared.\n\nJust FYI, in my private rewrite (that I'll discard), I made it like\nso:\n\n builtin/add.c:refresh() declares 'flags' as signed, and uses it\n to call refresh_index() that expects an unsigned value.\n\n> Since in this case 'flags' represents\n> a bag of bits, whose MSB is not used\n> in special ways, change the type\n> of 'flags' to unsigned.\n\nGood, too.\n\nWill queue.\n\n> Signed-off-by: Eugenio Gigante \n> ---\n> builtin/add.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ada7719561..393c10cbcf 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -115,7 +115,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)\n> \tint i, ret = 0;\n> \tchar *skip_worktree_seen = NULL;\n> \tstruct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;\n> -\tint flags = REFRESH_IGNORE_SKIP_WORKTREE |\n> +\tunsigned int flags = REFRESH_IGNORE_SKIP_WORKTREE |\n> \t\t (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);\n> \n> \tseen = xcalloc(pathspec->nr, 1);\n"}]}