[MPlayer-dev-eng] [PATCH] Use BUFLENGTH for buf defined in draw_image

[Reimar Döffinger]

On Mon, Dec 13, 2010 at 12:16:15PM -0200, Marco Aurélio Graciotto Silva wrote:
> On Mon, Dec 13, 2010 at 11:20 AM, Clément Bœsch <ubitux at gmail.com> wrote:
> > On Sun, Dec 12, 2010 at 03:26:38PM -0200, Marco Aurélio Graciotto Silva wrote:
> >
> >> Index: libvo/vo_png.c
> >> ===================================================================
> >> --- libvo/vo_png.c    (revision 32708)
> >> +++ libvo/vo_png.c    (working copy)
> >> @@ -129,13 +129,13 @@
> >>      AVFrame pic;
> >>      int buffersize;
> >>      int res;
> >> -    char buf[100];
> >> +    char buf[BUFLENGTH];
> >>      FILE *outfile;
> >>
> >>      // if -dr or -slices then do nothing:
> >>      if(mpi->flags&(MP_IMGFLAG_DIRECT|MP_IMGFLAG_DRAW_CALLBACK)) return VO_TRUE;
> >>
> >> -    snprintf (buf, 100, "%s/%08d.png", png_outdir, ++framenum);
> >> +    snprintf(buf, BUFLENGTH, "%s/%08d.png", png_outdir, ++framenum);
> >>      outfile = fopen(buf, "wb");
> >>      if (!outfile) {
> >>          mp_msg(MSGT_VO,MSGL_WARN, MSGTR_LIBVO_PNG_ErrorOpeningForWriting, strerror(errno));
> >
> > This looks ok to me, but you should use sizeof(buf) instead of reusing the
> > BUFLENGTH macro in snprintf.
> >
> > I'm also wondering if there is no other similar macro defined elsewhere
> > that could be use… But well, this is not that important.
> 
> Actually there is. vo_png.c is pretty similar to vo_jpeg.c and vo_pnm.c:
> both define BUFLENGTH, implements identical {pnm|jpeg|png}_mkdir
> functions, use the same strategy to define the filename
> (snprinf(buf,  BUFLENGTH...)).

And they're honestly all wrong. 100 is laughably short for a file name,
and the appropriate constant for all file paths is PATH_MAX (or is it
MAX_PATH? I can never remember).
Problem is that it's not that great to use it on the stack since it often
is 4kB (or more?), but MPlayer is doing that already anyway...