[FFmpeg-devel] [PATCH] parse_options(): Avoid passing NULL as a string arg to fprintf
stefano.sabatini-lala at poste.it
Fri Jun 24 11:12:59 CEST 2011
On date Friday 2011-06-24 04:48:29 +0200, Michael Niedermayer encoded:
> On Thu, Jun 23, 2011 at 02:57:45PM -0400, Jeff Downs wrote:
> > I recently restored FATE testing on SPARC/Solaris. One test is presently
> > failing, where ffmpeg crashes when given the argument -pix_fmts.
> > The crash occurs due to arg being NULL in the fprintf mentioned in the
> > patch below. The patch fixes the specific symptom, and seems to be a
> > good idea anyway as in the general case it appears that arg can be NULL.
> i agree, the patch LGTM
Yes, I relied on the GNU libc convention to print "(null)" when an
argument is NULL, I suppose this is not portable so the patch should
be fine. Can you say which libc are you using?
> > However this doesn't fix the underlying problem that when processing
> > -pix_fmts (and others like it), the error case is being executed to begin
> > with.
> > This is because several of the arg processing functions, the one for
> > -pix_fmts included, are accessed via function pointer int (*fp)(const char
> > *, const char *) but, in reality, are functions declared & implemented as
> > void f(void).
> > On sparc, executing these with the expectation of returning int does not
> > yield a 0 return (as it apparently does on other platforms).
> this bug sounds related to eb8bc57240d5d3e4680ff1df18a0a7792e96bd0c
> > I was going to send patches to fix this, too, but that would require
> > changing all the arg processing functions to match the pointer signature
> > and thought that it best to put the problem out there and collect any
> > comments/input before working on such a large change.
> > Note that most of the problematic functions would have to be changed to
> > both take args (where they don't presently, and hence the args would be
> > dummy) and to return a value (fixed 0) to fix this to be "proper". Input
> > welcome.
> ill be happy with whatever you and stefano agree to
I suppose the right fix would be to change the signatures of the
cmdutils.c:* function with the mismatching signatures, maybe prefix
them with "opt_" to make clear that they're opt functions.
FFmpeg = Faithless and Fascinating Murdering Plastic Eretic Guide
More information about the ffmpeg-devel