| Message ID | 20200525140801.18889-5-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | c33e56c7a6a8bef7d95e1d36eb2f35748d475695 |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avformat/aviobuf: Don't check for overflow after it happened | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index a13c677875..66cc8c7adc 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -1370,13 +1370,13 @@ int avio_get_dyn_buf(AVIOContext *s, uint8_t **pbuffer) { DynBuffer *d; - if (!s || s->error) { + if (!s) { *pbuffer = NULL; return 0; } d = s->opaque; - if (!d->size) { + if (!s->error && !d->size) { *pbuffer = d->io_buffer; return FFMAX(s->buf_ptr, s->buf_ptr_max) - s->buffer; }
Two kinds of errors can happen when working with dynamic buffers: (Re)allocation errors or truncation errors (one has to truncate the buffer to a size of INT_MAX because avio_close_dyn_buf() and avio_get_dyn_buf() both return an int). Right now, avio_get_dyn_buf() returns an empty buffer in either case. But given that avio_get_dyn_buf() does not destroy the dynamic buffer, one can return the buffer in case of truncation and let the user check the error flags and decide for himself instead of hardcoding a single way to proceed in case of truncation. (This actually restores the behaviour from before commit 163bb9ac0af495a5cb95441bdb5c02170440d28c.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- Unfortunately the "let the user decide" approach is not possible for avio_close_dyn_buf(). Said function has many more deficits: On allocation failure it returns a size of -AV_INPUT_BUFFER_PADDING_SIZE, although the documentation does not even allow negative return values at all. On truncation, the padding is not really written, yet it is subtracted from the size (or to put it another way: the padding is not zeroed in this case). It does not return the actual size of the buffer, but rather the last position (which can be different in case a backward seek had been performed). In this case, it is not the end of the buffer that is padded, but rather the position when calling avio_close_dyn_buf(). There is no documented way to check for errors; checking whether the returned buffer is NULL works in case of allocation failures, but not for truncation. If we returned NULL on truncation, too, callers could check for errors. But a real solution would involve deprecating avio_close_dyn_buf() and maybe the rest of the current dynamic buffer API. libavformat/aviobuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)