James Almer:
> It will be removed in the next commit.
>
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavcodec/utils.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index c08f9a7da3..5394a179b0 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -29,7 +29,7 @@
> #include "libavutil/avassert.h"
> #include "libavutil/avstring.h"
> #include "libavutil/intreadwrite.h"
> -#include "libavutil/mem_internal.h"
> +#include "libavutil/mem.h"
> #include "libavutil/pixdesc.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/pixfmt.h"
> @@ -49,25 +49,31 @@
>
> void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t min_size)
> {
> - uint8_t **p = ptr;
> + uint8_t *tmp, **p = ptr;
> if (min_size > SIZE_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
> av_freep(p);
> *size = 0;
> return;
> }
> - if (!ff_fast_malloc(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE, 1))
> + tmp = *p;
> + av_fast_mallocz(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE);
> + /* don't zero the padding if the buffer was reallocated */
> + if (*p && *p == tmp)
> memset(*p + min_size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
> }
>
> void av_fast_padded_mallocz(void *ptr, unsigned int *size, size_t min_size)
> {
> - uint8_t **p = ptr;
> + uint8_t *tmp, **p = ptr;
> if (min_size > SIZE_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
> av_freep(p);
> *size = 0;
> return;
> }
> - if (!ff_fast_malloc(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE, 1))
> + tmp = *p;
> + av_fast_mallocz(p, size, min_size + AV_INPUT_BUFFER_PADDING_SIZE);
> + /* don't zero the buffer if it was reallocated */
> + if (*p && *p == tmp)
> memset(*p, 0, min_size + AV_INPUT_BUFFER_PADDING_SIZE);
> }
>
>
The checks "*p == tmp" are flawed: It might be that the buffer has been
reallocated and that *p == tmp is nevertheless true (namely if the
buffer could have just been extended at the end) which might lead to
unnecessary zeroing; even worse, "[t]he value of a pointer becomes
indeterminate when the object it points to reaches the end of its
lifetime" (C99, 6.2.4.2), so it is compliant with the standard that if
av_fast_mallocz() reallocates the buffer, the tmp pointer on the stack
of av_fast_padded_malloc[z] might suddenly change (I don't know a system
in which this happens, though).
This can be avoided by always zeroing the padding in
av_fast_padded_malloc() and zeroing the buffer in
av_fast_padded_mallocz() ourselves (in which case one should use
av_fast_malloc() in av_fast_padded_mallocz()).
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
