[FFmpeg-devel,1/2] avformat/webm_chunk: Fix argument length of get_chunk_filename()

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/webm_chunk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Michael Niedermayer:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/webm_chunk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
> index 2c99753b5b..561ec152e7 100644
> --- a/libavformat/webm_chunk.c
> +++ b/libavformat/webm_chunk.c
> @@ -84,7 +84,7 @@ static int chunk_mux_init(AVFormatContext *s)
>      return 0;
>  }
>  
> -static int get_chunk_filename(AVFormatContext *s, int is_header, char *filename)
> +static int get_chunk_filename(AVFormatContext *s, int is_header, char filename[MAX_FILENAME_SIZE])
>  {
>      WebMChunkContext *wc = s->priv_data;
>      AVFormatContext *oc = wc->avf;
> 
1. This is not a fix, merely a cosmetic clarification. After all, this
change does not allow the compiler to infer that every pointer
corresponding to the filename argument will point to an array of at
least MAX_FILENAME_SIZE elements. (C99 added a static keyword for this.)

2. You could just as well remove the check for whether filename is
NULL as the new form makes it clear that this function should not be
called with a NULL pointer as filename. (Of course, compilers can (and
do) already optimize the check away by looking at both calls of this
function.)

- Andreas

On Fri, May 03, 2019 at 06:11:00AM +0000, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/webm_chunk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
> > index 2c99753b5b..561ec152e7 100644
> > --- a/libavformat/webm_chunk.c
> > +++ b/libavformat/webm_chunk.c
> > @@ -84,7 +84,7 @@ static int chunk_mux_init(AVFormatContext *s)
> >      return 0;
> >  }
> >  
> > -static int get_chunk_filename(AVFormatContext *s, int is_header, char *filename)
> > +static int get_chunk_filename(AVFormatContext *s, int is_header, char filename[MAX_FILENAME_SIZE])
> >  {
> >      WebMChunkContext *wc = s->priv_data;
> >      AVFormatContext *oc = wc->avf;
> > 
> 1. This is not a fix, merely a cosmetic clarification. After all, this
> change does not allow the compiler to infer that every pointer
> corresponding to the filename argument will point to an array of at
> least MAX_FILENAME_SIZE elements. (C99 added a static keyword for this.)

static analyzers can use such hints to detect violations
but the real intent here was that the human developer would see
from just looking at the argument that it has a implied size.
And that way to avoid a mistake

[...]

On Fri, May 03, 2019 at 06:11:00AM +0000, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavformat/webm_chunk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
> > index 2c99753b5b..561ec152e7 100644
> > --- a/libavformat/webm_chunk.c
> > +++ b/libavformat/webm_chunk.c
> > @@ -84,7 +84,7 @@ static int chunk_mux_init(AVFormatContext *s)
> >      return 0;
> >  }
> >  
> > -static int get_chunk_filename(AVFormatContext *s, int is_header, char *filename)
> > +static int get_chunk_filename(AVFormatContext *s, int is_header, char filename[MAX_FILENAME_SIZE])
> >  {
> >      WebMChunkContext *wc = s->priv_data;
> >      AVFormatContext *oc = wc->avf;
> > 
> 1. This is not a fix, merely a cosmetic clarification. After all, this
> change does not allow the compiler to infer that every pointer
> corresponding to the filename argument will point to an array of at
> least MAX_FILENAME_SIZE elements. (C99 added a static keyword for this.)
> 

> 2. You could just as well remove the check for whether filename is
> NULL as the new form makes it clear that this function should not be
> called with a NULL pointer as filename. (Of course, compilers can (and
> do) already optimize the check away by looking at both calls of this
> function.)

possible, but removing the check isnt truly related to this change

thx

[...]

On Fri, May 03, 2019 at 06:03:02PM +0200, Michael Niedermayer wrote:
> On Fri, May 03, 2019 at 06:11:00AM +0000, Andreas Rheinhardt wrote:
> > Michael Niedermayer:
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >  libavformat/webm_chunk.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/webm_chunk.c b/libavformat/webm_chunk.c
> > > index 2c99753b5b..561ec152e7 100644
> > > --- a/libavformat/webm_chunk.c
> > > +++ b/libavformat/webm_chunk.c
> > > @@ -84,7 +84,7 @@ static int chunk_mux_init(AVFormatContext *s)
> > >      return 0;
> > >  }
> > >  
> > > -static int get_chunk_filename(AVFormatContext *s, int is_header, char *filename)
> > > +static int get_chunk_filename(AVFormatContext *s, int is_header, char filename[MAX_FILENAME_SIZE])
> > >  {
> > >      WebMChunkContext *wc = s->priv_data;
> > >      AVFormatContext *oc = wc->avf;
> > > 
> > 1. This is not a fix, merely a cosmetic clarification. After all, this
> > change does not allow the compiler to infer that every pointer
> > corresponding to the filename argument will point to an array of at
> > least MAX_FILENAME_SIZE elements. (C99 added a static keyword for this.)
> 
> static analyzers can use such hints to detect violations
> but the real intent here was that the human developer would see
> from just looking at the argument that it has a implied size.
> And that way to avoid a mistake

will apply with a improved commit message

thanks

[...]