[FFmpeg-devel] [PATCH] tools: add ffescape utility

[Stefano Sabatini]

On date Tuesday 2012-10-23 01:28:31 +0200, Stefano Sabatini encoded:
> On date Monday 2012-10-22 17:47:26 +0200, Nicolas George encoded:
[...]
> > > +#define WHITESPACES " \n\t"
> > 
> > We really need to unify that.
> 
> Suggestions?
> 
> > > +
> > > +static char *escape(const char *src, const char *special_chars)
> > 
> > I believe you could make the program simpler if this function would take the
> > bprint buffer as argument: it would just have to add to it, and all errors
> > checks and finalization would go outside. Without that, it looks like you
> > need to check for out-of-memory twice: once before returning from escape,
> > and a second time after the call to escape.
> 
> My first idea was to implement a library function, but I preferred to
> delay this since I'm not yet sure about which features to implement
> (e.g. \   lazy escaping  \  or 'quoting'), and I wanted to avoid an
> header dependency on bprint.h. 
> 
> > > +{
> > > +    AVBPrint dst;
> > > +    char *ret;
> > > +
> > > +    av_bprint_init(&dst, 1, AV_BPRINT_SIZE_UNLIMITED);
> > > +    for (; *src; src++) {
> > > +        if ((special_chars && strchr(special_chars, *src)) ||
> > > +            strchr("'\\" WHITESPACES, *src))
> > > +            av_bprintf(&dst, "\\%c", *src);
> > > +        else
> > > +            av_bprint_chars(&dst, *src, 1);
> > 
> > I believe the escaping could be made more elegant: count the number of
> > special chars S and the number of single quotes Q, if S > kQ, escape using
> > single quotes (the single quotes themselves being escaped as '\'', therefore
> > k=3 is a good guess), else escape using backslashes.
> 
> Yes this is a good idea. I wonder if there are some circumstances
> where the user may want to favor one type of escaping or the other
> (and thus make it configurable).

Now there are three different escaping "modes".

[...]
> > > +        case 'i':
> > > +            infilename = optarg;
> > > +            break;
> > 
> 
> > Do we really need to duplicate the shell's features?
> 
> pipes are not always available, e.g. in cmd.exe, thus helps
> portability.

Also it's useful when debugging with gdb.

[...]

Updated, I'll push in a few days if I read no comments.
-- 
FFmpeg = Fundamentalist and Foolish Minimalistic Pron Evanescent Gargoyle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-tools-add-ffescape-utility.patch
Type: text/x-diff
Size: 8399 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121024/27c0530a/attachment.bin>