[FFmpeg-devel] Patch review: av_dict: add support for empty values

[Michael Niedermayer]

On Wed, Mar 04, 2015 at 02:24:58PM +0100, Michael Niedermayer wrote:
> On Wed, Mar 04, 2015 at 02:11:42PM +0100, Michael Niedermayer wrote:
> > On Tue, Mar 03, 2015 at 10:51:01PM -0400, Peter Cordes wrote:
> > [...]
> > > Anyway, the av_dict change is the one that requires the most review, so
> > > I'll make this email focus on that set of changes.
> > > https://github.com/FFmpeg/FFmpeg/pull/118
> > 
> > pull req #3, patch #1 review
> > 
> > > -    char *ret     = out, *end = out;
> > > +    char *ret     = out, *end_quote = out;
> > 
> > why ?
> > also cosmetical changes should, if they are done, be in seperate
> > patches
> > 
> > 
> > > -    while (*p && !strspn(p, term)) {
> > > +    while (*p && !strchr(term, *p)) {
> > 
> > this should be in a seperate patch
> > 
> > 
> 
> > > +    ret = av_realloc(ret, out - ret + 2);
> > > +    // if realloc fails to shrink, we're hosed anyway; just leak the old buffer
> > >     return ret;
> > 
> > if realloc fails to shrink, the original unshrunk array should be
> > returned to avoid the leak and failure
> 
> ahh, i see you fix this in a later commit, this should be stashed
> with the patch that would add the bug if it was pushed

and now i see you mentioned that in your mail, i guess replying to
the first part and then reviewing some of the code before reading
the rest of the mail was not really a great idea

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150304/afdac4da/attachment.asc>