[FFmpeg-devel] [PATCH] Remove size magic number (15) and use sizeof instead

[Ganesh Ajjanagadde]

On Tue, Nov 24, 2015 at 6:55 AM,  <webmaster at mohammadg.com> wrote:
> From: Mohammad Ghasembeigi <webmaster at mohammadg.com>
>
> ---
>  libavcodec/dsicinvideo.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/dsicinvideo.c b/libavcodec/dsicinvideo.c
> index f95cbc7..dc9ae7d 100644
> --- a/libavcodec/dsicinvideo.c
> +++ b/libavcodec/dsicinvideo.c
> @@ -100,12 +100,12 @@ 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);
> -    src += 15;
> +    memcpy(huff_code_table, src, sizeof(huff_code_table));
> +    src += sizeof(huff_code_table);
>
>      while (src < src_end) {
>          huff_code = *src++;
> -        if ((huff_code >> 4) == 15) {
> +        if ((huff_code >> 4) == sizeof(huff_code_table)) {
>              b          = huff_code << 4;
>              huff_code  = *src++;
>              *dst_cur++ = b | (huff_code >> 4);
> @@ -114,8 +114,8 @@ static int cin_decode_huffman(const unsigned char *src, int src_size,
>          if (dst_cur >= dst_end)
>              break;
>
> -        huff_code &= 15;
> -        if (huff_code == 15) {
> +        huff_code &= sizeof(huff_code_table);
> +        if (huff_code == sizeof(huff_code_table)) {
>              *dst_cur++ = *src++;
>          } else
>              *dst_cur++ = huff_code_table[huff_code];
> --
> 1.9.5.msysgit.1

This patch is really secondary and some here may disagree with it
(maybe the << 4, >> 4 already implicity assumes 2^4 -1?), though I
don't mind it. Until the actual point by Clement is addressed via
another patch, I am not too happy.

There are a number of ways to deal with Clement's point, but the best
ways will require careful study. For instance, should the code return
an error with too small a packet, or should something else be done?
Answer depends on the larger context.

>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel