[PATCH] Use BUFLENGTH for buf defined in draw_image
In libvo/vo_png.c, the variable 'buf', used to create the output PNG filename, has a constant size of 100. However, vo_png.c already defines and uses a constant, BUFLENGTH, for a similar purpose at config(). So, I changed draw_image() to use BUFLENGTH instead of 100 as its size. Regards, Marco
On Sun, Dec 12, 2010 at 03:26:38PM -0200, Marco Aurélio Graciotto Silva wrote:
In libvo/vo_png.c, the variable 'buf', used to create the output PNG filename, has a constant size of 100. However, vo_png.c already defines and uses a constant, BUFLENGTH, for a similar purpose at config(). So, I changed draw_image() to use BUFLENGTH instead of 100 as its size.
Regards,
Marco
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. -- Clément B. Not sent from a jesusPhone.
On Mon, Dec 13, 2010 at 11:20 AM, Clément Bœsch <ubitux@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...)). As a next step, I can merge the code that is similar between them and fix this issue about the buffer size. -- Marco Aurélio
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@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...
On Mon, Dec 13, 2010 at 12:16:15PM -0200, Marco Aurélio Graciotto Silva wrote:
As a next step, I can merge the code that is similar between them and fix this issue about the buffer size.
Please do. Diego
participants (4)
-
Clément Bœsch -
Diego Biurrun -
Marco Aurélio Graciotto Silva -
Reimar Döffinger