[FFmpeg-devel] [PACTH] Fix another problem in av_set_string()

[Stefano Sabatini]

On date Tuesday 2008-05-13 13:16:25 +0200, Michael Niedermayer encoded:
> On Tue, May 13, 2008 at 11:38:34AM +0200, Stefano Sabatini wrote:
> > On date Tuesday 2008-05-13 10:18:47 +0200, Michael Niedermayer encoded:
> > > On Tue, May 13, 2008 at 09:38:58AM +0200, Stefano Sabatini wrote:
> > > > Hi all,
> > > > 
> > > > since evaluation occurs on the buffer buf:
> > > > d = ff_eval2(buf, const_values, const_names, NULL, NULL, NULL, NULL, NULL, &error);
> > > > 
> > > > then avlog should report it when telling there was a parsing error here:
> > > > if (error)
> > > >    av_log(NULL, AV_LOG_ERROR, "Unable to parse option value \"%s\": %s\n", val, error);
> > > >                                                                            ^^^
> > > > while the val contains the remaining part of the argument string still
> > > > to be evaluated.
> > > > 
> > > > This pach does s/val/buf/ in the abovementioned line.
> > > 
> > > ive rejected this patch already
> > > 
> > > the bug is that val+=i is done too early.
> > 
> > mmh.. got it, please check this one.
> 
> rejected

I think you rejected the patch because it didn't follow strictly
your suggestion (to move the position of val+=i), so I'm attaching a
patch corresponding to what I suppose is your preferred solution (or
maybe you're just annoyed by my profusion of trivial patches with
corresponding trivial errors, who knows?).

Nonetheless I think that my solution issues clearer output (the whole
argument value is output, as opposed to just the misbehaving chop
followed by the remaining part of the value), but I know by experience
that your eyes are sharper than mine so maybe you just catched some
error in my patch which I overlooked.

In order to support my thesis I'm attaching a test script with the
*manually edited* output corresponding to the three possible cases (no
patch, fix-av-set-string-01.patch and fix-av-set-string-02.patch),
please say which you prefer if any.

Sorry for the many attachments, regards.
-- 
FFmpeg = Friendly and Fucking MultiPurpose EnGine
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-av-set-string-01.patch
Type: text/x-diff
Size: 1111 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080513/4eca87c2/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-av-set-string-02.patch
Type: text/x-diff
Size: 652 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080513/4eca87c2/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avopt-test.sh
Type: application/x-sh
Size: 437 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080513/4eca87c2/attachment.sh>
-------------- next part --------------
stefano at geppetto ~/s/ffmpeg> avopt-test.sh

./ffmpeg -flags -mv4+obmc+qpel+foo-trell+none
Unable to parse option value "-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -flags -+obmc+bar+qpel+foo-trell+none
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -b foo+1
Unable to parse option value "+1": missing (
Must supply at least one output file

./ffmpeg -b 100*foo-1
Unable to parse option value "-1": missing (
Must supply at least one output file

./ffmpeg -bt -100
Value -100.000000 for parameter 'bt' out of range.
Must supply at least one output file

./ffmpeg -bt foo
Unable to parse option value "": missing (
./ffmpeg: unrecognized option '-bt'

./ffmpeg -bt 1000+foo
Unable to parse option value "": missing (
./ffmpeg: unrecognized option '-bt'
-------------- next part --------------
stefano at geppetto ~/s/ffmpeg> avopt-test.sh

./ffmpeg -flags -mv4+obmc+qpel+foo-trell+none
Unable to parse option value "-mv4+obmc+qpel+foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -flags -+obmc+bar+qpel+foo-trell+none
Unable to parse option value "-+obmc+bar+qpel+foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -b foo+1
Unable to parse option value "foo+1": missing (
Must supply at least one output file

./ffmpeg -b 100*foo-1
Unable to parse option value "100*foo-1": missing (
Must supply at least one output file

./ffmpeg -bt -100
Value -100.000000 for parameter 'bt' out of range.
Must supply at least one output file

./ffmpeg -bt foo
Unable to parse option value "foo": missing (
./ffmpeg: unrecognized option '-bt'

./ffmpeg -bt 1000+foo
Unable to parse option value "1000+foo": missing (
./ffmpeg: unrecognized option '-bt'
-------------- next part --------------
stefano at geppetto ~/s/ffmpeg> avopt-test.sh

./ffmpeg -flags -mv4+obmc+qpel+foo-trell+none
Unable to parse option value "foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -flags -+obmc+bar+qpel+foo-trell+none
Unable to parse option value "+obmc+bar+qpel+foo-trell+none": missing (
./ffmpeg: unrecognized option '-flags'

./ffmpeg -b foo+1
Unable to parse option value "foo+1": missing (
Must supply at least one output file

./ffmpeg -b 100*foo-1
Unable to parse option value "100*foo-1": missing (
Must supply at least one output file

./ffmpeg -bt -100
Value -100.000000 for parameter 'bt' out of range.
Must supply at least one output file

./ffmpeg -bt foo
Unable to parse option value "foo": missing (
./ffmpeg: unrecognized option '-bt'

./ffmpeg -bt 1000+foo
Unable to parse option value "foo": missing (
./ffmpeg: unrecognized option '-bt'