[FFmpeg-devel] [PATCH] avformat/mp3enc: Assert that the header we assembled is valid

Ronald S. Bultje rsbultje at gmail.com
Sat Feb 6 14:42:00 CET 2016


Hi,

On Sat, Feb 6, 2016 at 5:32 AM, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Fri, Feb 05, 2016 at 10:41:26AM -0500, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Fri, Feb 5, 2016 at 9:34 AM, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
> >
> > > On Fri, Feb 5, 2016 at 3:33 PM, Derek Buitenhuis
> > > <derek.buitenhuis at gmail.com> wrote:
> > > > On 2/5/2016 2:19 PM, Michael Niedermayer wrote:
> > > >> because the code builds the header a few lines above, i assume
> > > >> the header we just build is correct
> > > >
> > > > Perhaps you could elaborate what CID1351343 is then, in the commit
> > > > messages.
> > > >
> > >
> > > A Coverity ID, a false positive which we decide to "fix" with an
> > > assertion instead of marking it as such in the tool?
> >
> >
> > So, I don't mind the patch, that's fine. But the commit message is
> > misleading. It suggests that there's a bug and that this patch fixes the
> > bug. That's incorrect. There is no bug, and this patch does not fix
> > anything. It asserts something, and as asserts go, they don't fix
> anything,
> > they just assert (=confirm) expected behaviour.
> >
> > So, can we change the commit message to not include the word "fix" or any
> > other misleading derivative thereof?
>
> absolutely, agree
> do you want to suggest some wording ?


I like Hendrik's earlier wording, something along the lines of "Makes
false-positive CID1351343 disappear", or "Related to false-positive
CID1351343".

Ronald


More information about the ffmpeg-devel mailing list