[FFmpeg-devel] [PATCH] lavu: add escape API
Nicolas George
nicolas.george at normalesup.org
Thu Jan 10 19:33:35 CET 2013
Le nonidi 19 nivôse, an CCXXI, Stefano Sabatini a écrit :
> >From 1e67bf4479879ad45da88e005578d56952ccde33 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Sun, 16 Dec 2012 12:17:23 +0100
> Subject: [PATCH] lavu: add escape API
>
> The escape API will be useful to perform escaping programmatically, which
> is required when crafting argument strings, and will be used for context
> printing as well.
>
> This is based on the ffescape tool code, with a few extensions and fixes.
>
> TODO: bump minor, add APIchanges entry
> ---
> libavutil/avstring.c | 18 ++++++++++
> libavutil/avstring.h | 37 +++++++++++++++++++-
> libavutil/bprint.c | 45 ++++++++++++++++++++++++
> libavutil/bprint.h | 15 ++++++++
> tools/ffescape.c | 92 +++++++++++---------------------------------------
> 5 files changed, 133 insertions(+), 74 deletions(-)
>
> diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> index 7072a55..35719fd 100644
> --- a/libavutil/avstring.c
> +++ b/libavutil/avstring.c
> @@ -28,6 +28,7 @@
> #include "config.h"
> #include "common.h"
> #include "mem.h"
> +#include "bprint.h"
>
> int av_strstart(const char *str, const char *pfx, const char **ptr)
> {
> @@ -251,6 +252,23 @@ const char *av_dirname(char *path)
> return path;
> }
>
> +int av_escape(char **dst, const char *src, const char *special_chars,
> + enum AVEscapeMode mode, int flags)
> +{
> + AVBPrint dstbuf;
> +
> + av_bprint_init(&dstbuf, 1, AV_BPRINT_SIZE_UNLIMITED);
> +
> + av_bprint_escape(&dstbuf, src, special_chars, mode, flags);
> + if (!av_bprint_is_complete(&dstbuf)) {
> + av_bprint_finalize(&dstbuf, NULL);
> + return AVERROR(ENOMEM);
> + } else {
> + int len = dstbuf.len;
> + av_bprint_finalize(&dstbuf, dst);
> + return len;
> + }
av_bprint_finalize will fail if the buffer is truncated, so you do not have
to check for it. Also, it will not change dstbuf.len: you do not have to
keep a copy of it.
> +}
>
> #ifdef TEST
>
> diff --git a/libavutil/avstring.h b/libavutil/avstring.h
> index d7af9ec..f909c12 100644
> --- a/libavutil/avstring.h
> +++ b/libavutil/avstring.h
> @@ -202,7 +202,6 @@ int av_strcasecmp(const char *a, const char *b);
> */
> int av_strncasecmp(const char *a, const char *b, size_t n);
>
> -
Spurious but harmless.
> /**
> * Thread safe basename.
> * @param path the path, on DOS both \ and / are considered separators.
> @@ -218,6 +217,42 @@ const char *av_basename(const char *path);
> */
> const char *av_dirname(char *path);
>
> +enum AVEscapeMode {
> + AV_ESCAPE_MODE_BACKSLASH = 1, ///< Use backslash escaping.
> + AV_ESCAPE_MODE_QUOTE = 2, ///< Use single-quote escaping.
> +};
> +
> +/**
> + * Consider spaces special and escape them even in the middle of the
> + * string.
> + * This is equivalent to adding the whitespace characters to the special
> + * characters lists, except it is guaranteed to use the exact same list
> + * of whitespace characters as the rest of libavutil.
> + */
> +#define AV_ESCAPE_FLAG_WHITESPACE 0x01
> +
> +/**
> + * Escape only specified special characters.
> + * Without this flag, escape also any characters that may be considered
> + * special by av_get_token(), such as the single quote.
> + */
> +#define AV_ESCAPE_FLAG_STRICT 0x02
> +
> +/**
> + * Escape string in src, and put the escaped string in an allocated
> + * string in *dst, which must be freed with av_free().
> + *
> + * @param dst pointer where an allocated string is put
> + * @param src string to escape, must be non-NULL
> + * @param special_chars string containing the special characters which need to be escaped
> + * @param mode escape mode to employ, see AV_ESCAPE_MODE_ macros
> + * @param flags flags which control how to escape, see AV_ESCAPE_FLAG_ macros
> + * @return the length of the allocated string, or a negative error code in case of error
> + * @see av_bprint_escape()
> + */
> +int av_escape(char **dst, const char *src, const char *special_chars,
> + enum AVEscapeMode mode, int flags);
I really think the flags and mode could be the same argument, but as you
prefer.
Also, there is no "do for the best" mode, where the function selects between
\ and '' and nothing (and possible other future escaping methods) depending
on the string.
> +
> /**
> * @}
> */
> diff --git a/libavutil/bprint.c b/libavutil/bprint.c
> index 4684ab4..64cd8db 100644
> --- a/libavutil/bprint.c
> +++ b/libavutil/bprint.c
> @@ -23,6 +23,7 @@
> #include <string.h>
> #include <time.h>
> #include "avassert.h"
> +#include "avstring.h"
> #include "bprint.h"
> #include "common.h"
> #include "error.h"
> @@ -217,6 +218,50 @@ int av_bprint_finalize(AVBPrint *buf, char **ret_str)
> return ret;
> }
>
> +#define WHITESPACES " \n\t"
> +
> +int av_bprint_escape(AVBPrint *dstbuf, const char *src, const char *special_chars,
> + enum AVEscapeMode mode, int flags)
> +{
> + switch (mode) {
> + case AV_ESCAPE_MODE_BACKSLASH:
> + /* \-escape characters */
> +
> + /* escape initial whitespace */
> + if (!(flags & AV_ESCAPE_FLAG_STRICT) && strchr(WHITESPACES, *src))
> + av_bprintf(dstbuf, "\\%c", *src++);
> +
> + for (; *src; src++) {
> + int is_last = !*(src+1);
> + if ((is_last && !(flags & AV_ESCAPE_FLAG_STRICT) && strchr(WHITESPACES, *src)) ||
> + (special_chars && strchr(special_chars, *src)) ||
> + strchr("'\\", *src) ||
> + (flags & AV_ESCAPE_FLAG_WHITESPACE && strchr(WHITESPACES, *src)))
I wonder whether this big condition could be simplified by using a byte- or
bit-array:
char special[256] = { 0 };
for (p = special_chars; *p; p++) special[*p] = 1;
if (!(flags & AV_ESCAPE_FLAG_STRICT))
special['\\'] = special['\''] = 1;
if (flags & AV_ESCAPE_FLAG_WHITESPACE)
special[' '] = special['\t'] = 1;
Also, I wonder whether the last statement of the condition will not escape
the first space twice.
> + av_bprint_chars(dstbuf, '\\', 1);
> + av_bprint_chars(dstbuf, *src, 1);
> + }
> + break;
> +
> + case AV_ESCAPE_MODE_QUOTE:
> + /* enclose the string between '' */
> + av_bprint_chars(dstbuf, '\'', 1);
> + for (; *src; src++) {
> + if (*src == '\'')
> + av_bprintf(dstbuf, "'\\''");
> + else
> + av_bprint_chars(dstbuf, *src, 1);
> + }
> + av_bprint_chars(dstbuf, '\'', 1);
> + break;
> +
> + default:
> + /* unknown escape mode */
> + return AVERROR(EINVAL);
> + }
> +
> + return dstbuf->len;
Why return anything? My philosophy for bprint is: add anything to it, do not
check for errors, check for errors at the end. Having all functions void
shows this.
The only error case here, apart from truncation inside bprint, is an invalid
mode selected. It is a programmatic error, not an environmental one, so a
less convenient way of reporting it would not be a problem.
> +}
> +
> #ifdef TEST
>
> #undef printf
> diff --git a/libavutil/bprint.h b/libavutil/bprint.h
> index f3915fe..b9039bd 100644
> --- a/libavutil/bprint.h
> +++ b/libavutil/bprint.h
> @@ -22,6 +22,7 @@
> #define AVUTIL_BPRINT_H
>
> #include "attributes.h"
> +#include "avstring.h"
>
> /**
> * Define a structure with extra padding to a fixed size
> @@ -180,4 +181,18 @@ static inline int av_bprint_is_complete(AVBPrint *buf)
> */
> int av_bprint_finalize(AVBPrint *buf, char **ret_str);
>
> +/**
> + * Escape the content in src and append it to dstbuf.
> + *
> + * @param dstbuf already inited destination bprint buffer
> + * @param src string containing the text to escape
> + * @param special_chars list of special characters to escape
> + * @param mode escape mode to employ, see AV_ESCAPE_MODE_* macros
> + * @param flags flags which control how to escape, see AV_ESCAPE_FLAG_* macros
> + * @return the length of the buffered string so far, a negative value in case of error
> + * @see av_escape()
> + */
> +int av_bprint_escape(AVBPrint *dstbuf, const char *src, const char *special_chars,
> + enum AVEscapeMode mode, int flags);
> +
> #endif /* AVUTIL_BPRINT_H */
> diff --git a/tools/ffescape.c b/tools/ffescape.c
> index d777fe4..cd2f56f 100644
> --- a/tools/ffescape.c
> +++ b/tools/ffescape.c
> @@ -42,80 +42,16 @@ static void usage(void)
> printf("\n"
> "Options:\n"
> "-e echo each input line on output\n"
> + "-f flag select an escape flag, can assume the values 'whitespace' and 'strict'\n"
> "-h print this help\n"
> "-i INFILE set INFILE as input file, stdin if omitted\n"
> "-l LEVEL set the number of escaping levels, 1 if omitted\n"
> - "-m ESCAPE_MODE select escape mode between 'full', 'lazy', 'quote', default is 'lazy'\n"
> + "-m ESCAPE_MODE select escape mode between 'backslash', 'quote', default is 'backslash'\n"
> "-o OUTFILE set OUTFILE as output file, stdout if omitted\n"
> "-p PROMPT set output prompt, is '=> ' by default\n"
> "-s SPECIAL_CHARS set the list of special characters\n");
> }
>
> -#define WHITESPACES " \n\t"
> -
> -enum EscapeMode {
> - ESCAPE_MODE_FULL,
> - ESCAPE_MODE_LAZY,
> - ESCAPE_MODE_QUOTE,
> -};
> -
> -static int escape(char **dst, const char *src, const char *special_chars,
> - enum EscapeMode mode)
> -{
> - AVBPrint dstbuf;
> -
> - av_bprint_init(&dstbuf, 1, AV_BPRINT_SIZE_UNLIMITED);
> -
> - switch (mode) {
> - case ESCAPE_MODE_FULL:
> - case ESCAPE_MODE_LAZY:
> - /* \-escape characters */
> -
> - if (mode == ESCAPE_MODE_LAZY && strchr(WHITESPACES, *src))
> - av_bprintf(&dstbuf, "\\%c", *src++);
> -
> - for (; *src; src++) {
> - if ((special_chars && strchr(special_chars, *src)) ||
> - strchr("'\\", *src) ||
> - (mode == ESCAPE_MODE_FULL && strchr(WHITESPACES, *src)))
> - av_bprintf(&dstbuf, "\\%c", *src);
> - else
> - av_bprint_chars(&dstbuf, *src, 1);
> - }
> -
> - if (mode == ESCAPE_MODE_LAZY && strchr(WHITESPACES, dstbuf.str[dstbuf.len-1])) {
> - char c = dstbuf.str[dstbuf.len-1];
> - dstbuf.str[dstbuf.len-1] = '\\';
> - av_bprint_chars(&dstbuf, c, 1);
> - }
> - break;
> -
> - case ESCAPE_MODE_QUOTE:
> - /* enclose between '' the string */
> - av_bprint_chars(&dstbuf, '\'', 1);
> - for (; *src; src++) {
> - if (*src == '\'')
> - av_bprintf(&dstbuf, "'\\''");
> - else
> - av_bprint_chars(&dstbuf, *src, 1);
> - }
> - av_bprint_chars(&dstbuf, '\'', 1);
> - break;
> -
> - default:
> - /* unknown escape mode */
> - return AVERROR(EINVAL);
> - }
> -
> - if (!av_bprint_is_complete(&dstbuf)) {
> - av_bprint_finalize(&dstbuf, NULL);
> - return AVERROR(ENOMEM);
> - } else {
> - av_bprint_finalize(&dstbuf, dst);
> - return 0;
> - }
> -}
> -
> int main(int argc, char **argv)
> {
> AVBPrint src;
> @@ -123,13 +59,14 @@ int main(int argc, char **argv)
> const char *outfilename = NULL, *infilename = NULL;
> FILE *outfile = NULL, *infile = NULL;
> const char *prompt = "=> ";
> - enum EscapeMode escape_mode = ESCAPE_MODE_LAZY;
> + enum AVEscapeMode escape_mode = AV_ESCAPE_MODE_BACKSLASH;
> + int escape_flags = 0;
> int level = 1;
> int echo = 0;
> char *special_chars = NULL;
> int c;
>
> - while ((c = getopt(argc, argv, "ehi:l:o:m:p:s:")) != -1) {
> + while ((c = getopt(argc, argv, "ef:hi:l:o:m:p:s:")) != -1) {
> switch (c) {
> case 'e':
> echo = 1;
> @@ -140,6 +77,16 @@ int main(int argc, char **argv)
> case 'i':
> infilename = optarg;
> break;
> + case 'f':
> + if (!strcmp(optarg, "whitespace")) escape_flags |= AV_ESCAPE_FLAG_WHITESPACE;
> + else if (!strcmp(optarg, "strict")) escape_flags |= AV_ESCAPE_FLAG_STRICT;
> + else {
> + av_log(NULL, AV_LOG_ERROR,
> + "Invalid value '%s' for option -f, "
> + "valid arguments are 'whitespace', and 'strict'\n", optarg);
> + return 1;
> + }
> + break;
> case 'l':
> {
> char *tail;
> @@ -154,13 +101,12 @@ int main(int argc, char **argv)
> break;
> }
> case 'm':
> - if (!strcmp(optarg, "full")) escape_mode = ESCAPE_MODE_FULL;
> - else if (!strcmp(optarg, "lazy")) escape_mode = ESCAPE_MODE_LAZY;
> - else if (!strcmp(optarg, "quote")) escape_mode = ESCAPE_MODE_QUOTE;
> + if (!strcmp(optarg, "backslash")) escape_mode = AV_ESCAPE_MODE_BACKSLASH;
> + else if (!strcmp(optarg, "quote")) escape_mode = AV_ESCAPE_MODE_QUOTE;
> else {
> av_log(NULL, AV_LOG_ERROR,
> "Invalid value '%s' for option -m, "
> - "valid arguments are 'full', 'lazy', 'quote'\n", optarg);
> + "valid arguments are 'backslash', and 'quote'\n", optarg);
> return 1;
> }
> break;
> @@ -219,7 +165,7 @@ int main(int argc, char **argv)
> /* escape */
> dst_buf = src_buf;
> while (level--) {
> - if (escape(&dst_buf, src_buf, special_chars, escape_mode) < 0) {
> + if (av_escape(&dst_buf, src_buf, special_chars, escape_mode, escape_flags) < 0) {
> av_log(NULL, AV_LOG_ERROR, "Could not escape string\n");
> return 1;
> }
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130110/dfea8bab/attachment.asc>
More information about the ffmpeg-devel
mailing list