[FFmpeg-devel] [PATCH] parse_options(): Avoid passing NULL as a string arg to fprintf

[Stefano Sabatini]

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