[FFmpeg-devel] [PATCH] replace magic number with size of array

Mohammad G webmaster at mohammadg.com
Mon Nov 23 18:34:28 CET 2015


On Tue, Nov 24, 2015 at 4:10 AM, Ganesh Ajjanagadde <gajjanag at mit.edu>
wrote:

> On Mon, Nov 23, 2015 at 12:03 PM,  <webmaster at mohammadg.com> wrote:
> > From: Mohammad Ghasembeigi <webmaster at mohammadg.com>
> >
> > ---
> >  libavcodec/dsicinvideo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/dsicinvideo.c b/libavcodec/dsicinvideo.c
> > index f95cbc7..cb5baf8 100644
> > --- a/libavcodec/dsicinvideo.c
> > +++ b/libavcodec/dsicinvideo.c
> > @@ -100,7 +100,7 @@ static int cin_decode_huffman(const unsigned char
> *src, int src_size,
> >      unsigned char *dst_end       = dst + dst_size;
> >      const unsigned char *src_end = src + src_size;
> >
> > -    memcpy(huff_code_table, src, 15);
> > +    memcpy(huff_code_table, src, sizeof(huff_code_table));
>
> Please address Clement's point. How do you know that src has at least
> 15 elements? Possibly use src_size here.
>
> >      src += 15;
>
> Since you are doing the change to the sizeof, change other usages of
> the 15 to a sizeof.
> It may also be useful to separate the 2 things into separate patches:
> 1. Check size of source.
> 2. 15 -> sizeof.
>
> >
> >      while (src < src_end) {
> > --
> > 1.9.5.msysgit.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Clement makes a good point, src can potentially be smaller than 15 elements.
What would be best to do in this case? (as the function should never be
called in such cases)

> Since you are doing the change to the sizeof, change other usages of
> the 15 to a sizeof.
> It may also be useful to separate the 2 things into separate patches:
> 1. Check size of source.
> 2. 15 -> sizeof.

Agreed, there are multiple magic numbers in the same function that could be
put in the same patch.


More information about the ffmpeg-devel mailing list