| Message ID | 20221122031739.3440-1-kunyu@nfschina.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | builtin: submodule--helper: Add allocation check of module_cb structure objects may be helpful in case of allocation failure | expand |
Li kunyu <kunyu@nfschina.com> writes: > The purpose of using temp is unknown, but adding an allocation failure > report should be helpful. Yuck. Use of xmalloc() may be more appropriate. Use of BUG() in a context like this is never the right thing to do. BUG() is reserved for cases where the condition should NEVER hold true in a correctly written program, and even for a correctly written program, malloc() can fail after you allocated too much memory from the heap. > > Signed-off-by: Li kunyu <kunyu@nfschina.com> > --- > builtin/submodule--helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index b63f420ece..2e379623ea 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -1077,6 +1077,9 @@ static void submodule_summary_callback(struct diff_queue_struct *q, > if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode)) > continue; > temp = (struct module_cb*)malloc(sizeof(struct module_cb)); > + if (!temp) > + BUG("The module_cb structure object fails to be allocated and the program may terminate abnormally"); > + > temp->mod_src = p->one->mode; > temp->mod_dst = p->two->mode; > temp->oid_src = p->one->oid;
On 11/22/22 10:17, Li kunyu wrote: > The purpose of using temp is unknown, but adding an allocation failure > report should be helpful. > Don't you understand the code? > Signed-off-by: Li kunyu <kunyu@nfschina.com> > --- > builtin/submodule--helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index b63f420ece..2e379623ea 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -1077,6 +1077,9 @@ static void submodule_summary_callback(struct diff_queue_struct *q, > if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode)) > continue; > temp = (struct module_cb*)malloc(sizeof(struct module_cb)); > + if (!temp) > + BUG("The module_cb structure object fails to be allocated and the program may terminate abnormally"); > + Why do you check whenever allocating temp is successful?
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b63f420ece..2e379623ea 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1077,6 +1077,9 @@ static void submodule_summary_callback(struct diff_queue_struct *q, if (!S_ISGITLINK(p->one->mode) && !S_ISGITLINK(p->two->mode)) continue; temp = (struct module_cb*)malloc(sizeof(struct module_cb)); + if (!temp) + BUG("The module_cb structure object fails to be allocated and the program may terminate abnormally"); + temp->mod_src = p->one->mode; temp->mod_dst = p->two->mode; temp->oid_src = p->one->oid;
The purpose of using temp is unknown, but adding an allocation failure report should be helpful. Signed-off-by: Li kunyu <kunyu@nfschina.com> --- builtin/submodule--helper.c | 3 +++ 1 file changed, 3 insertions(+)