avpacket: Initialize the allocated padding area in side data
diff mbox series

Message ID 20180201130237.87681-1-martin@martin.st
State Committed
Headers show
Series
  • avpacket: Initialize the allocated padding area in side data
Related show

Commit Message

Martin Storsjö Feb. 1, 2018, 1:02 p.m. 
This makes sure that consumers of the side data actually can
rely on the padding as intended, without having the callers of
av_packet_new_side_data to explicitly zero initialize it.
---
 libavcodec/avpacket.c | 1 +
 1 file changed, 1 insertion(+)

Comments

wm4 Feb. 1, 2018, 1:06 p.m. | #1 
On Thu,  1 Feb 2018 15:02:37 +0200
Martin Storsjö <martin@martin.st> wrote:

> This makes sure that consumers of the side data actually can
> rely on the padding as intended, without having the callers of
> av_packet_new_side_data to explicitly zero initialize it.
> ---
>  libavcodec/avpacket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 93e9eb6ae7..c705df3d59 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>      data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>      if (!data)
>          return NULL;
> +    memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>  
>      ret = av_packet_add_side_data(pkt, type, data, size);
>      if (ret < 0) {

+1, though I also have trouble believing that just clearing the entire
thing (including side data contents) would cause any kind of performance
trouble. (I.e. using av_mallocz.)

(Disregard my other empty reply I possibly sent, not sure.)
Martin Storsjö Feb. 1, 2018, 1:09 p.m. | #2 
On Thu, 1 Feb 2018, wm4 wrote:

> On Thu,  1 Feb 2018 15:02:37 +0200
> Martin Storsjö <martin@martin.st> wrote:
>
>> This makes sure that consumers of the side data actually can
>> rely on the padding as intended, without having the callers of
>> av_packet_new_side_data to explicitly zero initialize it.
>> ---
>>  libavcodec/avpacket.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index 93e9eb6ae7..c705df3d59 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>>      data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>      if (!data)
>>          return NULL;
>> +    memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>
>>      ret = av_packet_add_side_data(pkt, type, data, size);
>>      if (ret < 0) {
>
> +1, though I also have trouble believing that just clearing the entire
> thing (including side data contents) would cause any kind of performance
> trouble. (I.e. using av_mallocz.)

Yes, that'd probably also be just as nice. The upside with this approach 
is that tools more easily can point out if you as a caller forgot to 
initialize the data you actually asked to have allocated for you, but I 
don't have a very strong opinion either way.

// Martin
Luca Barbato Feb. 1, 2018, 1:45 p.m. | #3 
On 01/02/2018 14:02, Martin Storsjö wrote:
> This makes sure that consumers of the side data actually can
> rely on the padding as intended, without having the callers of
> av_packet_new_side_data to explicitly zero initialize it.
> ---
>   libavcodec/avpacket.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 93e9eb6ae7..c705df3d59 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -271,6 +271,7 @@ uint8_t *av_packet_new_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>       data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>       if (!data)
>           return NULL;
> +    memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>   
>       ret = av_packet_add_side_data(pkt, type, data, size);
>       if (ret < 0) {
> 

Fine for me.

Patch
diff mbox series 

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 93e9eb6ae7..c705df3d59 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -271,6 +271,7 @@  uint8_t *av_packet_new_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
     data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
     if (!data)
         return NULL;
+    memset(data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
 
     ret = av_packet_add_side_data(pkt, type, data, size);
     if (ret < 0) {