All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: rick.chang@mediatek.com, linux-mediatek@lists.infradead.org
Cc: linux-media@vger.kernel.org
Subject: [bug report] [media] vcodec: mediatek: Add Mediatek JPEG Decoder Driver
Date: Thu, 17 Mar 2022 14:21:18 +0300	[thread overview]
Message-ID: <20220317112118.GA29101@kili> (raw)

Hello Media Devs!

The patch b2f0d2724ba4: "[media] vcodec: mediatek: Add Mediatek JPEG
Decoder Driver" from Dec 14, 2016, leads to the following Smatch
static checker warning:

	drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_parse.c:140 mtk_jpeg_do_parse()
	warn: duplicate check 'notfound' (previous on line 67)

drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_parse.c
    57 static bool mtk_jpeg_do_parse(struct mtk_jpeg_dec_param *param, u8 *src_addr_va,
    58                               u32 src_size)
    59 {
    60         bool notfound = true;
    61         struct mtk_jpeg_stream stream;
    62 
    63         stream.addr = src_addr_va;
    64         stream.size = src_size;
    65         stream.curr = 0;
    66 
    67         while (notfound) {
    68                 int i, length, byte;
    69                 u32 word;
    70 
    71                 byte = read_byte(&stream);
    72                 if (byte == -1)
    73                         return false;
    74                 if (byte != 0xff)
    75                         continue;
    76                 do
    77                         byte = read_byte(&stream);
    78                 while (byte == 0xff);
    79                 if (byte == -1)
    80                         return false;
    81                 if (byte == 0)
    82                         continue;
    83 
    84                 length = 0;
    85                 switch (byte) {
    86                 case SOF0:
    87                         /* length */
    88                         if (read_word_be(&stream, &word))
    89                                 break;
    90 
    91                         /* precision */
    92                         if (read_byte(&stream) == -1)
    93                                 break;
    94 
    95                         if (read_word_be(&stream, &word))
    96                                 break;
    97                         param->pic_h = word;
    98 
    99                         if (read_word_be(&stream, &word))
    100                                 break;
    101                         param->pic_w = word;
    102 
    103                         param->comp_num = read_byte(&stream);
    104                         if (param->comp_num != 1 && param->comp_num != 3)
    105                                 break;
    106 
    107                         for (i = 0; i < param->comp_num; i++) {
    108                                 param->comp_id[i] = read_byte(&stream);
    109                                 if (param->comp_id[i] == -1)
    110                                         break;
    111 
    112                                 /* sampling */
    113                                 byte = read_byte(&stream);
    114                                 if (byte == -1)
    115                                         break;
    116                                 param->sampling_w[i] = (byte >> 4) & 0x0F;
    117                                 param->sampling_h[i] = byte & 0x0F;
    118 
    119                                 param->qtbl_num[i] = read_byte(&stream);
    120                                 if (param->qtbl_num[i] == -1)
    121                                         break;
    122                         }
    123 
    124                         notfound = !(i == param->comp_num);

Should this:

	if (i == param->comp_num)
		return true;

    125                         break;
    126                 case RST ... RST + 7:
    127                 case SOI:
    128                 case EOI:
    129                 case TEM:
    130                         break;
    131                 default:
    132                         if (read_word_be(&stream, &word))
    133                                 break;
    134                         length = (long)word - 2;
    135                         read_skip(&stream, length);
    136                         break;
    137                 }
    138         }
    139 
--> 140         return !notfound;

We know that "notfound" is false.  The double negatives confuse me.  I
guess that means it must be found here?

    141 }

regards,
dan carpenter

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: rick.chang@mediatek.com, linux-mediatek@lists.infradead.org
Cc: linux-media@vger.kernel.org
Subject: [bug report] [media] vcodec: mediatek: Add Mediatek JPEG Decoder Driver
Date: Thu, 17 Mar 2022 14:21:18 +0300	[thread overview]
Message-ID: <20220317112118.GA29101@kili> (raw)

Hello Media Devs!

The patch b2f0d2724ba4: "[media] vcodec: mediatek: Add Mediatek JPEG
Decoder Driver" from Dec 14, 2016, leads to the following Smatch
static checker warning:

	drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_parse.c:140 mtk_jpeg_do_parse()
	warn: duplicate check 'notfound' (previous on line 67)

drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_parse.c
    57 static bool mtk_jpeg_do_parse(struct mtk_jpeg_dec_param *param, u8 *src_addr_va,
    58                               u32 src_size)
    59 {
    60         bool notfound = true;
    61         struct mtk_jpeg_stream stream;
    62 
    63         stream.addr = src_addr_va;
    64         stream.size = src_size;
    65         stream.curr = 0;
    66 
    67         while (notfound) {
    68                 int i, length, byte;
    69                 u32 word;
    70 
    71                 byte = read_byte(&stream);
    72                 if (byte == -1)
    73                         return false;
    74                 if (byte != 0xff)
    75                         continue;
    76                 do
    77                         byte = read_byte(&stream);
    78                 while (byte == 0xff);
    79                 if (byte == -1)
    80                         return false;
    81                 if (byte == 0)
    82                         continue;
    83 
    84                 length = 0;
    85                 switch (byte) {
    86                 case SOF0:
    87                         /* length */
    88                         if (read_word_be(&stream, &word))
    89                                 break;
    90 
    91                         /* precision */
    92                         if (read_byte(&stream) == -1)
    93                                 break;
    94 
    95                         if (read_word_be(&stream, &word))
    96                                 break;
    97                         param->pic_h = word;
    98 
    99                         if (read_word_be(&stream, &word))
    100                                 break;
    101                         param->pic_w = word;
    102 
    103                         param->comp_num = read_byte(&stream);
    104                         if (param->comp_num != 1 && param->comp_num != 3)
    105                                 break;
    106 
    107                         for (i = 0; i < param->comp_num; i++) {
    108                                 param->comp_id[i] = read_byte(&stream);
    109                                 if (param->comp_id[i] == -1)
    110                                         break;
    111 
    112                                 /* sampling */
    113                                 byte = read_byte(&stream);
    114                                 if (byte == -1)
    115                                         break;
    116                                 param->sampling_w[i] = (byte >> 4) & 0x0F;
    117                                 param->sampling_h[i] = byte & 0x0F;
    118 
    119                                 param->qtbl_num[i] = read_byte(&stream);
    120                                 if (param->qtbl_num[i] == -1)
    121                                         break;
    122                         }
    123 
    124                         notfound = !(i == param->comp_num);

Should this:

	if (i == param->comp_num)
		return true;

    125                         break;
    126                 case RST ... RST + 7:
    127                 case SOI:
    128                 case EOI:
    129                 case TEM:
    130                         break;
    131                 default:
    132                         if (read_word_be(&stream, &word))
    133                                 break;
    134                         length = (long)word - 2;
    135                         read_skip(&stream, length);
    136                         break;
    137                 }
    138         }
    139 
--> 140         return !notfound;

We know that "notfound" is false.  The double negatives confuse me.  I
guess that means it must be found here?

    141 }

regards,
dan carpenter

             reply	other threads:[~2022-03-17 11:21 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-17 11:21 Dan Carpenter [this message]
2022-03-17 11:21 ` [bug report] [media] vcodec: mediatek: Add Mediatek JPEG Decoder Driver Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220317112118.GA29101@kili \
    --to=dan.carpenter@oracle.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=rick.chang@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.