{"thread":{"id":"58836","subject":"[PATCH] builtin: submodule--helper: Add allocation check of module_cb structure objects may be helpful in case of allocation failure","startedAt":"2022-11-22T03:17:52Z","lastAt":"2022-11-22T08:08:27Z","messageCount":3,"participants":["Li kunyu","Junio C Hamano","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"467744","messageId":"20221122031739.3440-1-kunyu@nfschina.com","threadId":"58836","inReplyTo":null,"subject":"[PATCH] builtin: submodule--helper: Add allocation check of module_cb structure objects may be helpful in case of allocation failure","fromName":"Li kunyu","fromEmail":"kunyu@nfschina.com","sentAt":"2022-11-22T03:17:39Z","receivedAt":"2022-11-22T03:17:52Z","isPatch":true,"sender":{"key":"kunyu@nfschina.com","avatar":null},"body":"The purpose of using temp is unknown, but adding an allocation failure\nreport should be helpful.\n\nSigned-off-by: Li kunyu <kunyu@nfschina.com>\n---\n builtin/submodule--helper.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b63f420ece..2e379623ea 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1077,6 +1077,9 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n \t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n+\t\tif (!temp)\n+\t\t\t BUG(\"The module_cb structure object fails to be allocated and the program may terminate abnormally\");\n+\n \t\ttemp->mod_src = p->one->mode;\n \t\ttemp->mod_dst = p->two->mode;\n \t\ttemp->oid_src = p->one->oid;\n-- \n2.18.2\n\n"},{"id":"467747","messageId":"xmqqh6yrsjmu.fsf@gitster.g","threadId":"58836","inReplyTo":"20221122031739.3440-1-kunyu@nfschina.com","subject":"Re: [PATCH] builtin: submodule--helper: Add allocation check of module_cb structure objects may be helpful in case of allocation failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-22T04:22:49Z","receivedAt":"2022-11-22T04:23:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Li kunyu <kunyu@nfschina.com> writes:\n\n> The purpose of using temp is unknown, but adding an allocation failure\n> report should be helpful.\n\nYuck.  Use of xmalloc() may be more appropriate.\n\nUse of BUG() in a context like this is never the right thing to do.\nBUG() is reserved for cases where the condition should NEVER hold\ntrue in a correctly written program, and even for a correctly\nwritten program, malloc() can fail after you allocated too much\nmemory from the heap.\n\n>\n> Signed-off-by: Li kunyu <kunyu@nfschina.com>\n> ---\n>  builtin/submodule--helper.c | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index b63f420ece..2e379623ea 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1077,6 +1077,9 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n>  \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n>  \t\t\tcontinue;\n>  \t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n> +\t\tif (!temp)\n> +\t\t\t BUG(\"The module_cb structure object fails to be allocated and the program may terminate abnormally\");\n> +\n>  \t\ttemp->mod_src = p->one->mode;\n>  \t\ttemp->mod_dst = p->two->mode;\n>  \t\ttemp->oid_src = p->one->oid;\n"},{"id":"467755","messageId":"6c81f4b0-065f-01db-09ef-6ad9e3b0b39c@gmail.com","threadId":"58836","inReplyTo":"20221122031739.3440-1-kunyu@nfschina.com","subject":"Re: [PATCH] builtin: submodule--helper: Add allocation check of module_cb structure objects may be helpful in case of allocation failure","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2022-11-22T08:08:17Z","receivedAt":"2022-11-22T08:08:27Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 11/22/22 10:17, Li kunyu wrote:\n> The purpose of using temp is unknown, but adding an allocation failure\n> report should be helpful.\n> \n\nDon't you understand the code?\n\n> Signed-off-by: Li kunyu <kunyu@nfschina.com>\n> ---\n>  builtin/submodule--helper.c | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index b63f420ece..2e379623ea 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1077,6 +1077,9 @@ static void submodule_summary_callback(struct diff_queue_struct *q,\n>  \t\tif (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode))\n>  \t\t\tcontinue;\n>  \t\ttemp = (struct module_cb*)malloc(sizeof(struct module_cb));\n> +\t\tif (!temp)\n> +\t\t\t BUG(\"The module_cb structure object fails to be allocated and the program may terminate abnormally\");\n> +\n\nWhy do you check whenever allocating temp is successful?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n\n"}]}