Message ID | 20190502184936.8818-1-michael@niedermayer.cc |
---|---|
State | Accepted |
Commit | 1a74b04737f08e2e11a02ada280407889f6cadb1 |
Headers | show |
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 [...]
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;
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/webm_chunk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)