[FFmpeg-devel] [PATCH] Implement read_file() cmdutils.c option

[Stefano Sabatini]

On date Tuesday 2010-03-30 15:22:54 +0200, Michael Niedermayer encoded:
> On Tue, Mar 30, 2010 at 01:38:02AM +0200, Stefano Sabatini wrote:
> [...]> diff --git a/cmdutils.c b/cmdutils.c
> > index 1ffe2e8..f561316 100644
> > --- a/cmdutils.c
> > +++ b/cmdutils.c
> > @@ -639,3 +639,26 @@ int read_yesno(void)
> >  
> >      return yesno;
> >  }
> > +
> 
> > +int read_file(const char *filename, char **bufptr, int *size)
> 
> int is the wrong type for an array size

Yes, size_t looks more correct, that's also what is returned by fread(). 
> 
> > +{
> > +    FILE *f = fopen(filename, "r");
> > +
> > +    if (!f) {
> > +        fprintf(stderr, "Cannot read file '%s': %s\n", filename, strerror(errno));
> > +        return AVERROR(errno);
> > +    }
> > +    fseek(f, 0, SEEK_END);
> 
> > -                    size = ftell(f);
> > +    *size = ftell(f) + 1;
> 
> you mess +-1 up

Well like this or either I don't care.

> [...]
> > diff --git a/cmdutils.h b/cmdutils.h
> > index 9190a81..6d12a97 100644
> > --- a/cmdutils.h
> > +++ b/cmdutils.h
> > @@ -200,4 +200,15 @@ void show_pix_fmts(void);
> >   */
> >  int read_yesno(void);
> >  
> > +/**
> > + * Reads the file with name filename, and puts its content in a newly
> > + * allocated 0-terminated buffer.
> > + *
> > + * @param bufptr puts here the pointer to the newly allocated buffer
> > + * @param size puts here the size of the newly allocated buffer
> > + * @return 0 in case of success, a negative value corresponding to an
> > + * AVERROR error code in case of failure.
> 
> why not return the size?

size_t is unsigned, returning a negative error code looks nicer and
easier to integrate.

Regards.
-- 
FFmpeg = Fierce Formidable Maxi Proud Ephemeral Guru