diff mbox series

[FFmpeg-devel,3/4] avutil/detection_bbox: simplify code for better readability

Message ID 1632836198-23044-3-git-send-email-lance.lmwang@gmail.com
State Accepted
Commit 9997047a1802b5174e423cf0863041639664ab87
Headers show
Series [FFmpeg-devel,1/4] avutil/detection_bbox: fix the memory leak on error | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Limin Wang Sept. 28, 2021, 1:36 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavutil/detection_bbox.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

Comments

Andreas Rheinhardt Sept. 28, 2021, 1:57 p.m. UTC | #1
lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavutil/detection_bbox.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/libavutil/detection_bbox.c b/libavutil/detection_bbox.c
> index 8bfdff7..3de66ce 100644
> --- a/libavutil/detection_bbox.c
> +++ b/libavutil/detection_bbox.c
> @@ -23,27 +23,29 @@ AVDetectionBBoxHeader *av_detection_bbox_alloc(uint32_t nb_bboxes, size_t *out_s
>      size_t size;
>      struct BBoxContext {
>          AVDetectionBBoxHeader header;
> -        AVDetectionBBox boxes[1];
> -    } *ret;
> +        AVDetectionBBox boxes;
> +    };
>      const size_t bboxes_offset = offsetof(struct BBoxContext, boxes);
> +    const size_t bbox_size = sizeof(AVDetectionBBox);
> +    AVDetectionBBoxHeader *header;
>  
> -    size = sizeof(*ret);
> -    if (nb_bboxes - 1 > (SIZE_MAX - size) / sizeof(*ret->boxes))

This here is actually not a simplification, but a patch: The old code
would not really work if nb_bboxes == 0.

> +    size = bboxes_offset;
> +    if (nb_bboxes > (SIZE_MAX - size) / bbox_size)
>          return NULL;
> -    size += sizeof(*ret->boxes) * (nb_bboxes - 1);
> +    size += bbox_size * nb_bboxes;
>  
> -    ret = av_mallocz(size);
> -    if (!ret)
> +    header = av_mallocz(size);
> +    if (!header)
>          return NULL;
>  
> -    ret->header.nb_bboxes = nb_bboxes;
> -    ret->header.bbox_size = sizeof(*ret->boxes);
> -    ret->header.bboxes_offset = bboxes_offset;
> +    header->nb_bboxes     = nb_bboxes;
> +    header->bbox_size     = bbox_size;
> +    header->bboxes_offset = bboxes_offset;
>  
>      if (out_size)
>          *out_size = size;
>  
> -    return &ret->header;
> +    return header;
>  }
>  
>  AVDetectionBBoxHeader *av_detection_bbox_create_side_data(AVFrame *frame, uint32_t nb_bboxes)
>
Limin Wang Sept. 28, 2021, 2:19 p.m. UTC | #2
On Tue, Sep 28, 2021 at 03:57:27PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavutil/detection_bbox.c | 24 +++++++++++++-----------
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavutil/detection_bbox.c b/libavutil/detection_bbox.c
> > index 8bfdff7..3de66ce 100644
> > --- a/libavutil/detection_bbox.c
> > +++ b/libavutil/detection_bbox.c
> > @@ -23,27 +23,29 @@ AVDetectionBBoxHeader *av_detection_bbox_alloc(uint32_t nb_bboxes, size_t *out_s
> >      size_t size;
> >      struct BBoxContext {
> >          AVDetectionBBoxHeader header;
> > -        AVDetectionBBox boxes[1];
> > -    } *ret;
> > +        AVDetectionBBox boxes;
> > +    };
> >      const size_t bboxes_offset = offsetof(struct BBoxContext, boxes);
> > +    const size_t bbox_size = sizeof(AVDetectionBBox);
> > +    AVDetectionBBoxHeader *header;
> >  
> > -    size = sizeof(*ret);
> > -    if (nb_bboxes - 1 > (SIZE_MAX - size) / sizeof(*ret->boxes))
> 
> This here is actually not a simplification, but a patch: The old code
> would not really work if nb_bboxes == 0.

OK, I'll change the comment message to "Fix the av_detection_bbox_alloc failed if nb_bboxes == 0"


> 
> > +    size = bboxes_offset;
> > +    if (nb_bboxes > (SIZE_MAX - size) / bbox_size)
> >          return NULL;
> > -    size += sizeof(*ret->boxes) * (nb_bboxes - 1);
> > +    size += bbox_size * nb_bboxes;
> >  
> > -    ret = av_mallocz(size);
> > -    if (!ret)
> > +    header = av_mallocz(size);
> > +    if (!header)
> >          return NULL;
> >  
> > -    ret->header.nb_bboxes = nb_bboxes;
> > -    ret->header.bbox_size = sizeof(*ret->boxes);
> > -    ret->header.bboxes_offset = bboxes_offset;
> > +    header->nb_bboxes     = nb_bboxes;
> > +    header->bbox_size     = bbox_size;
> > +    header->bboxes_offset = bboxes_offset;
> >  
> >      if (out_size)
> >          *out_size = size;
> >  
> > -    return &ret->header;
> > +    return header;
> >  }
> >  
> >  AVDetectionBBoxHeader *av_detection_bbox_create_side_data(AVFrame *frame, uint32_t nb_bboxes)
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavutil/detection_bbox.c b/libavutil/detection_bbox.c
index 8bfdff7..3de66ce 100644
--- a/libavutil/detection_bbox.c
+++ b/libavutil/detection_bbox.c
@@ -23,27 +23,29 @@  AVDetectionBBoxHeader *av_detection_bbox_alloc(uint32_t nb_bboxes, size_t *out_s
     size_t size;
     struct BBoxContext {
         AVDetectionBBoxHeader header;
-        AVDetectionBBox boxes[1];
-    } *ret;
+        AVDetectionBBox boxes;
+    };
     const size_t bboxes_offset = offsetof(struct BBoxContext, boxes);
+    const size_t bbox_size = sizeof(AVDetectionBBox);
+    AVDetectionBBoxHeader *header;
 
-    size = sizeof(*ret);
-    if (nb_bboxes - 1 > (SIZE_MAX - size) / sizeof(*ret->boxes))
+    size = bboxes_offset;
+    if (nb_bboxes > (SIZE_MAX - size) / bbox_size)
         return NULL;
-    size += sizeof(*ret->boxes) * (nb_bboxes - 1);
+    size += bbox_size * nb_bboxes;
 
-    ret = av_mallocz(size);
-    if (!ret)
+    header = av_mallocz(size);
+    if (!header)
         return NULL;
 
-    ret->header.nb_bboxes = nb_bboxes;
-    ret->header.bbox_size = sizeof(*ret->boxes);
-    ret->header.bboxes_offset = bboxes_offset;
+    header->nb_bboxes     = nb_bboxes;
+    header->bbox_size     = bbox_size;
+    header->bboxes_offset = bboxes_offset;
 
     if (out_size)
         *out_size = size;
 
-    return &ret->header;
+    return header;
 }
 
 AVDetectionBBoxHeader *av_detection_bbox_create_side_data(AVFrame *frame, uint32_t nb_bboxes)