577207 - vp8 warning: empty body in an if-statement

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[timeless]

media/libvpx/vp8/

decoder/decodemv.c: In function ‘vp8_decode_mode_mvs’:
decoder/decodemv.c:235: warning: empty body in an if-statement

Comment 1

[timeless]

Attachment: patch

Comment 2

[timeless]

i've filed a bug for upstream:
http://code.google.com/p/webm/issues/detail?id=123

Comment 3

[Michael Kohler [:mkohler]]

Attachment: Patch v2

Comment 4

[Michael Kohler [:mkohler]]

Attachment: Patch v3

.. sorry about this spam ..

Comment 5

[Timothy B. Terriberry (:derf)]

Comment on attachment 456291 [details] [diff] [review]
Patch v3

Personally I would be the entire if block inside #if CONFIG_DEBUG, but I don't think that's actually important.

Comment 6

[Michael Kohler [:mkohler]]

Could you please write a commit message for committing this to the upstream? Honestly, I have no idea what this patch is about, so I need your help.

BTW: timeless, do you want any credits for this bug in the commit message?

Comment 7

[timeless]

[e-mail] is sufficient

Comment 8

[timeless]

err, wait...

[e-mail] for author in realm:*.mozilla.org

[e-mail] for author if it's upstream

Comment 9

[timeless]

so, derf's right, the whole block is useless.

can you redo the patch as:

+                         while (j != L[++k])
++                        {
++#if CONFIG_DEBUG
+                             if (k >= 16)
++                            {
+-#if CONFIG_DEBUG
+                                 assert(0);
+-
+-#else
+-                                ;
++                            }
+ #endif
++                        }

the commit message should probably be something like:

limit range checking code for L[k] to CONFIG_DEBUG

Comment 10

[Michael Kohler [:mkohler]]

Attachment: Patch v4

I will upload this patch to the libvpx repository later this evening.

Comment 11

[Michael Kohler [:mkohler]]

Still no verification upstream.

derf: could you get someone to do that? ;)

Comment 12

[Michael Kohler [:mkohler]]

BTW: http://review.webmproject.org/317

Comment 13

[Timothy B. Terriberry (:derf)]

Upstream has been kicked and the patch merged.

Comment 14

[Michael Kohler [:mkohler]]

(In reply to comment #13)
> Upstream has been kicked and the patch merged.

Thanks a lot.I don't think we want to wait till the next libvpx update, so I think we need to checkin that ourself.Should be a low risk checkin AFAIK.

Comment 15

[Timothy B. Terriberry (:derf)]

Well, only blockers are being allowed until Fx 4, and I don't think warning fixes count as that.

Comment 16

[Michael Kohler [:mkohler]]

(In reply to comment #15)
> Well, only blockers are being allowed until Fx 4, and I don't think warning
> fixes count as that.

Oh, right. Forgot about the discussion in moz.dev.planning [1].. But does this mean we just land blockers till Fx 4 and no new bug fixes that make a real difference and are low risk patches? Frankly, this doesn't seem very reasonable to me since the Fx 4 release is still a few month away. Therefore asking for approval-2.0.

Comment 17

[Michael Kohler [:mkohler]]

Comment on attachment 456869 [details] [diff] [review]
Patch v4

this is a low risk change. I guess we could take it, but it's not my decision eventually.

Comment 18

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/mozilla-central/rev/a4b7ad2f4854