| Message ID | 1433425302-26448-3-git-send-email-yszhou4tech@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On 2015-06-04 15:41, Yousong Zhou wrote: > - Return early on calloc() failure. > - Correct comment text for __calloc_a(). > > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> > --- > utils.c | 4 ++++ > utils.h | 6 +++--- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/utils.c b/utils.c > index 8fd19f4..627b0f6 100644 > --- a/utils.c > +++ b/utils.c > @@ -43,6 +43,10 @@ void *__calloc_a(size_t len, ...) > va_end(ap1); > > ptr = calloc(1, alloc_len); > + if (!ptr) { > + va_end(ap); > + return NULL; > + } What's the point? The return value without this check will be NULL anyway, and optimizing a rare error case does not seem useful to me. - Felix
On Jun 5, 2015 3:14 PM, "Felix Fietkau" <nbd@openwrt.org> wrote: > > On 2015-06-04 15:41, Yousong Zhou wrote: > > - Return early on calloc() failure. > > - Correct comment text for __calloc_a(). > > > > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> > > --- > > utils.c | 4 ++++ > > utils.h | 6 +++--- > > 2 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/utils.c b/utils.c > > index 8fd19f4..627b0f6 100644 > > --- a/utils.c > > +++ b/utils.c > > @@ -43,6 +43,10 @@ void *__calloc_a(size_t len, ...) > > va_end(ap1); > > > > ptr = calloc(1, alloc_len); > > + if (!ptr) { > > + va_end(ap); > > + return NULL; > > + } > What's the point? The return value without this check will be NULL > anyway, and optimizing a rare error case does not seem useful to me. > rare case indeed, also a trivial change. null check is for correctness. it also seems to me assigning invalid pointer values to output arguments is not right. well, my 2 cents :) yousong > - Felix
On 2015-06-05 17:03, Yousong Zhou wrote: > > On Jun 5, 2015 3:14 PM, "Felix Fietkau" <nbd@openwrt.org > <mailto:nbd@openwrt.org>> wrote: >> >> On 2015-06-04 15:41, Yousong Zhou wrote: >> > - Return early on calloc() failure. >> > - Correct comment text for __calloc_a(). >> > >> > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com > <mailto:yszhou4tech@gmail.com>> >> > --- >> > utils.c | 4 ++++ >> > utils.h | 6 +++--- >> > 2 files changed, 7 insertions(+), 3 deletions(-) >> > >> > diff --git a/utils.c b/utils.c >> > index 8fd19f4..627b0f6 100644 >> > --- a/utils.c >> > +++ b/utils.c >> > @@ -43,6 +43,10 @@ void *__calloc_a(size_t len, ...) >> > va_end(ap1); >> > >> > ptr = calloc(1, alloc_len); >> > + if (!ptr) { >> > + va_end(ap); >> > + return NULL; >> > + } >> What's the point? The return value without this check will be NULL >> anyway, and optimizing a rare error case does not seem useful to me. >> > > rare case indeed, also a trivial change. null check is for > correctness. it also seems to me assigning invalid pointer values to > output arguments is not right. well, my 2 cents :) Well, the way you implemented it, the output argument values will be undefined, which is not significantly better :) - Felix
diff --git a/utils.c b/utils.c index 8fd19f4..627b0f6 100644 --- a/utils.c +++ b/utils.c @@ -43,6 +43,10 @@ void *__calloc_a(size_t len, ...) va_end(ap1); ptr = calloc(1, alloc_len); + if (!ptr) { + va_end(ap); + return NULL; + } alloc_len = 0; foreach_arg(ap, cur_addr, cur_len, &ret, len) { *cur_addr = &ptr[alloc_len]; diff --git a/utils.h b/utils.h index d00e76b..9456f72 100644 --- a/utils.h +++ b/utils.h @@ -25,8 +25,10 @@ #include <stdbool.h> #include <time.h> +#define calloc_a(len, ...) __calloc_a(len, ##__VA_ARGS__, NULL) + /* - * calloc_a(size_t len, [void **addr, size_t len,...], NULL) + * __calloc_a(size_t len, [void **addr, size_t len,...], NULL) * * allocate a block of memory big enough to hold multiple aligned objects. * the pointer to the full object (starting with the first chunk) is returned, @@ -34,8 +36,6 @@ * the last argument needs to be a NULL pointer */ -#define calloc_a(len, ...) __calloc_a(len, ##__VA_ARGS__, NULL) - void *__calloc_a(size_t len, ...); #ifndef ARRAY_SIZE
- Return early on calloc() failure. - Correct comment text for __calloc_a(). Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> --- utils.c | 4 ++++ utils.h | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-)