[FFmpeg-devel] [PATCH] Make cmdutils.c:print_error() use strerror() if av_strerror() fails (e.g. if strerror_r() is not defined)

Stefano Sabatini stefano.sabatini-lala
Wed May 5 23:49:02 CEST 2010


On date Wednesday 2010-05-05 02:20:36 +0200, Michael Niedermayer encoded:
> On Wed, May 05, 2010 at 12:37:52AM +0200, Stefano Sabatini wrote:
> > On date Tuesday 2010-05-04 17:12:24 +0200, Michael Niedermayer encoded:
[...]
> > > > > missing {} and you should document the thread saftey issues tis introduces
> > > > > users of the code likely want to know
> > > > 
> > > > Uhm it's a cmdutils.c function, I don't think we need to document
> > > > this. Note also that the usage in ffplay of print_error() looks safe
> > > > anyway, since print_error() is used only in the init phase, with just
> > > > one active thread.
> > > 
> > > you need to document it if you want me to approve it ;)
> > 
> > Revisited patch and documentation for print_error() attached.
> > 
> > Regards.
> > -- 
> > FFmpeg = Fiendish and Friendly Multimedia Purposeless Ecumenical God
> 
> >  cmdutils.c |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 8e674f8a79111f0d7264d7827a430278287a03fb  0002-Make-print_error-use-strerror-in-case-av_strerror-fa.patch
> > >From a079441a9dbdca615cac8badf376ad9c97e4d3eb Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > Date: Mon, 3 May 2010 23:36:21 +0200
> > Subject: [PATCH 2/6] Make print_error() use strerror() in case av_strerror() fails.
> > 
> > Should provide a meaningful error message for systems which do not
> > support strerror_r().
> > 
> > Fix roundup issue #1894.
> > ---
> >  cmdutils.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> ok

Applied.

> [...]
> >  cmdutils.h |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 386c4acd4be5a4e8b73567c40fb89a3164d67089  0003-Document-cmdutils.c-print_error.patch
> > >From f41624ff126f9ee41a2ac9dbaef6c49d0a103a1d Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > Date: Wed, 5 May 2010 00:11:57 +0200
> > Subject: [PATCH 3/6] Document cmdutils.c:print_error().
> > 
> > ---
> >  cmdutils.h |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/cmdutils.h b/cmdutils.h
> > index ad8cefd..86da06c 100644
> > --- a/cmdutils.h
> > +++ b/cmdutils.h
> > @@ -134,6 +134,14 @@ void parse_options(int argc, char **argv, const OptionDef *options,
> >  
> >  void set_context_opts(void *ctx, void *opts_ctx, int flags);
> >  
> > +/**
> > + * Prints a string indicating filename and a description of the error
> > + * code err to stderr.
> 
> this sounds a bit uneasy and rough
> 
> what about
> Prints a human readable error message
> 
> 
> 
> > + * The use of this function may be unsafe in a multithreaded
> > + * application.
> 
> ... if have_strerwhatere_r is not available

Patch updated.
-- 
FFmpeg = Foolish and Frenzy MultiPurpose Extensive Gorilla



More information about the ffmpeg-devel mailing list