| Message ID | 20180214012749.43143-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | aa6280805e21e1e2c3fb7bb1efb47a27ceeb3fed |
| Headers | show |
On Tue, 13 Feb 2018 22:27:49 -0300 James Almer <jamrial@gmail.com> wrote: > This makes sure no field is ever used uninitialized. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > I think i prefer this one. > > libavformat/aviobuf.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c > index 86eb6579f4..d63db3897f 100644 > --- a/libavformat/aviobuf.c > +++ b/libavformat/aviobuf.c > @@ -87,6 +87,8 @@ int ffio_init_context(AVIOContext *s, > int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), > int64_t (*seek)(void *opaque, int64_t offset, int whence)) > { > + memset(s, 0, sizeof(AVIOContext)); > + > s->buffer = buffer; > s->orig_buffer_size = > s->buffer_size = buffer_size; > @@ -135,7 +137,7 @@ AVIOContext *avio_alloc_context( > int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), > int64_t (*seek)(void *opaque, int64_t offset, int whence)) > { > - AVIOContext *s = av_mallocz(sizeof(AVIOContext)); > + AVIOContext *s = av_malloc(sizeof(AVIOContext)); > if (!s) > return NULL; > ffio_init_context(s, buffer, buffer_size, write_flag, opaque, There are a lot of calls to ffio_init_context() - is it sure that there are no fields that must be preserved? I'd probably prefer "*s = (AVIOContext){0};", but that's just me.
On 2/14/2018 2:17 AM, wm4 wrote: > On Tue, 13 Feb 2018 22:27:49 -0300 > James Almer <jamrial@gmail.com> wrote: > >> This makes sure no field is ever used uninitialized. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> I think i prefer this one. >> >> libavformat/aviobuf.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c >> index 86eb6579f4..d63db3897f 100644 >> --- a/libavformat/aviobuf.c >> +++ b/libavformat/aviobuf.c >> @@ -87,6 +87,8 @@ int ffio_init_context(AVIOContext *s, >> int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), >> int64_t (*seek)(void *opaque, int64_t offset, int whence)) >> { >> + memset(s, 0, sizeof(AVIOContext)); >> + >> s->buffer = buffer; >> s->orig_buffer_size = >> s->buffer_size = buffer_size; >> @@ -135,7 +137,7 @@ AVIOContext *avio_alloc_context( >> int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), >> int64_t (*seek)(void *opaque, int64_t offset, int whence)) >> { >> - AVIOContext *s = av_mallocz(sizeof(AVIOContext)); >> + AVIOContext *s = av_malloc(sizeof(AVIOContext)); >> if (!s) >> return NULL; >> ffio_init_context(s, buffer, buffer_size, write_flag, opaque, > > There are a lot of calls to ffio_init_context() - is it sure that there > are no fields that must be preserved? I assume no. There are some fields that are currently not set by ffio_init_context(), including the checksum ones that created the problem at hand, but i don't think any should be preserved in case the context is reused. And for that matter, is there any code in libavformat that reuses these contexts at all? This init function is internal, so API users are unable to reuse AVIOContexts, and avio_alloc_context() always zero initializes them. > > I'd probably prefer "*s = (AVIOContext){0};", but that's just me. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 2/14/2018 2:29 PM, James Almer wrote: > On 2/14/2018 2:17 AM, wm4 wrote: >> On Tue, 13 Feb 2018 22:27:49 -0300 >> James Almer <jamrial@gmail.com> wrote: >> >>> This makes sure no field is ever used uninitialized. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> I think i prefer this one. >>> >>> libavformat/aviobuf.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c >>> index 86eb6579f4..d63db3897f 100644 >>> --- a/libavformat/aviobuf.c >>> +++ b/libavformat/aviobuf.c >>> @@ -87,6 +87,8 @@ int ffio_init_context(AVIOContext *s, >>> int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), >>> int64_t (*seek)(void *opaque, int64_t offset, int whence)) >>> { >>> + memset(s, 0, sizeof(AVIOContext)); >>> + >>> s->buffer = buffer; >>> s->orig_buffer_size = >>> s->buffer_size = buffer_size; >>> @@ -135,7 +137,7 @@ AVIOContext *avio_alloc_context( >>> int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), >>> int64_t (*seek)(void *opaque, int64_t offset, int whence)) >>> { >>> - AVIOContext *s = av_mallocz(sizeof(AVIOContext)); >>> + AVIOContext *s = av_malloc(sizeof(AVIOContext)); >>> if (!s) >>> return NULL; >>> ffio_init_context(s, buffer, buffer_size, write_flag, opaque, >> >> There are a lot of calls to ffio_init_context() - is it sure that there >> are no fields that must be preserved? > > I assume no. There are some fields that are currently not set by > ffio_init_context(), including the checksum ones that created the > problem at hand, but i don't think any should be preserved in case the > context is reused. > > And for that matter, is there any code in libavformat that reuses these > contexts at all? This init function is internal, so API users are unable > to reuse AVIOContexts, and avio_alloc_context() always zero initializes > them. Approved by wm4 on IRC, so pushed.
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 86eb6579f4..d63db3897f 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -87,6 +87,8 @@ int ffio_init_context(AVIOContext *s, int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), int64_t (*seek)(void *opaque, int64_t offset, int whence)) { + memset(s, 0, sizeof(AVIOContext)); + s->buffer = buffer; s->orig_buffer_size = s->buffer_size = buffer_size; @@ -135,7 +137,7 @@ AVIOContext *avio_alloc_context( int (*write_packet)(void *opaque, uint8_t *buf, int buf_size), int64_t (*seek)(void *opaque, int64_t offset, int whence)) { - AVIOContext *s = av_mallocz(sizeof(AVIOContext)); + AVIOContext *s = av_malloc(sizeof(AVIOContext)); if (!s) return NULL; ffio_init_context(s, buffer, buffer_size, write_flag, opaque,
This makes sure no field is ever used uninitialized. Signed-off-by: James Almer <jamrial@gmail.com> --- I think i prefer this one. libavformat/aviobuf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)