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

[Michael Niedermayer]

On Mon, Oct 15, 2012 at 10:14:47PM +0200, Stefano Sabatini wrote:
> On date Monday 2012-10-15 16:33:10 +0200, Michael Niedermayer encoded:
> > On Mon, Oct 15, 2012 at 12:24:41PM +0200, Stefano Sabatini wrote:
> > [...]
> > > +            if (*tail || li > INT_MAX || li < 0) {
> > > +                av_log(NULL, AV_LOG_ERROR,
> > > +                        "Invalid value '%s' for option -l, argument must be a non negative integer\n",
> > > +                        optarg);
> > > +                return 1;
> > > +            }
> > > +            level = li;
> > > +            break;
> > > +        }
> > > +        case 'o':
> > > +            outfilename = optarg;
> > > +            break;
> > > +        case 'p':
> > > +            prompt = optarg;
> > > +            break;
> > > +        case 's':
> > > +            special_chars = optarg;
> > > +            break;
> > > +        case '?':
> > > +            return 1;
> > > +        }
> > > +    }
> > > +
> > > +    if (!infilename || !strcmp(infilename, "-"))
> > > +        infilename = "/dev/stdin";
> > > +    infile = fopen(infilename, "r");
> > > +    if (!infile) {
> > > +        fprintf(stderr, "Impossible to open input file '%s': %s\n", infilename, strerror(errno));
> > 
> > is it intended that av_log and fprintf() are mixed ?
> 
> Changed in favor of av_log.
> 
> > 
> > 
> > > +        return 1;
> > > +    }
> > > +
> > 
> > > +    if (!outfilename || !strcmp(outfilename, "-"))
> > > +        outfilename = "/dev/stdout";
> > 
> > that wont work on windows
> 
> Changed.
>   
> > > +    outfile = fopen(outfilename, "w");
> > > +    if (!outfile) {
> > > +        fprintf(stderr, "Impossible to open output file '%s': %s\n", outfilename, strerror(errno));
> > > +        return 1;
> > > +    }
> > > +
> > > +    /* grab the input and store it in buf */
> > 
> > > +    while ((c = fgetc(infile)) != EOF) {
> > 
> > with this a 0xFF will be interpreted as EOF i think
> 
> char -> int;
>       
> > > +        if (count >= src_buf_size-1) {
> > > +            if (src_buf_size == MAX_BLOCK_SIZE) {
> > > +                av_log(NULL, AV_LOG_ERROR, "Memory allocation problem, "
> > > +                       "max block size '%zd' reached\n", MAX_BLOCK_SIZE);
> > > +                return 1;
> > > +            }
> > > +            src_buf_size = FFMIN(src_buf_size, MAX_BLOCK_SIZE / 2) * 2;
> > > +            src_buf = av_realloc_f((void *)src_buf, src_buf_size, 1);
> > > +            if (!src_buf) {
> > > +                av_log(NULL, AV_LOG_ERROR, "Memory allocation problem occurred\n");
> > > +                return 1;
> > > +            }
> > > +        }
> > > +        src_buf[count++] = c;
> > > +    }
> > > +    src_buf[count] = 0;
> > > +
> > > +    if (echo)
> > > +        fprintf(outfile, "%s ", src_buf);
> > > +
> > > +    /* escape */
> > > +    dst_buf = src_buf;
> > > +    while (level--) {
> > > +        dst_buf = escape(src_buf, special_chars);
> > > +        av_free(src_buf);
> > > +        src_buf = dst_buf;
> > 
> > missing null check on dst_buf
> 
> Fixed.
> 
> Updated.
[...]
> +int main(int argc, char **argv)
> +{
> +    size_t src_buf_size = 256;
> +    char *src_buf = av_malloc(src_buf_size), *dst_buf = NULL;

missing malloc failure check

no further comments from me / LGTM if there are no other comments

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- 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/20121016/916f8a56/attachment.asc>