[MPlayer-cvslog] r35626 - trunk/libaf/af_export.c

[Xidorn Quan]

On Sun, Dec 9, 2012 at 4:31 PM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de>wrote:

> On Sun, Dec 09, 2012 at 04:00:47PM +0800, Xidorn Quan wrote:
> > On Sun, Dec 9, 2012 at 3:17 PM, Reimar Döffinger
> > <Reimar.Doeffinger at gmx.de>wrote:
> >
> > > On 9 Dec 2012, at 03:22, upsuper <subversion at mplayerhq.hu> wrote:
> > >
> > > > Author: upsuper
> > > > Date: Sun Dec  9 03:22:08 2012
> > > > New Revision: 35626
> > > >
> > > > Log:
> > > > Fix a wrong condition.
> > >
> > > Those conditions were exactly right (allowing to dynamically allocate
> the
> > > buf array), they are completely pointless the way you changed them
> since
> > > free () handles it.
> > >
> >
> > I don't agree with you for the point that the conditions were right.
> >
> > The definition of s->buf is "void *buf[AF_NCH]" which means that buf
> > is an array which contains AF_NCH void pointers. Therefore, the size
> > of s->buf is immutable, and its address is always equal to that of
> > the struct plus an offset, which indicates that the value of s->buf
> > here can never equal to a non-zero value except s has a specific
> > value.
>
> The point is that you could disable the static AF_NCH limit by
> changing the definition to void **buf without risking crashes
> (though still a memleak).
> It is overkill and/or not such a great idea usually, but it can
> future-proof the code in some cases (probably not in this).
>

When that future comes, adding these conditions then might be a good
idea. Anyway, these conditions are not so useful for the current code,
and cause some warning from Coverity.

I'll remove the conditions.

-- 
Xidorn Quan
GnuPG fingerprint: 6F1E DF9A D250 7505 63E2  345E 7570 8D3F 7C9A 1209