diff mbox series

[FFmpeg-devel,v2,1/3] avformat/argo_asf: cleanup and NULL-terminate name field in header

Message ID 20211012110224.639056-1-zane@zanevaniperen.com
State Accepted
Commit 20fa838da5a1cce23cb571781fc7fae4b18146ae
Headers show
Series [FFmpeg-devel,v2,1/3] avformat/argo_asf: cleanup and NULL-terminate name field in header | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Zane van Iperen Oct. 12, 2021, 11:02 a.m. UTC 
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/argo_asf.c | 8 ++++----
 libavformat/argo_asf.h | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

Comments

Zane van Iperen Oct. 14, 2021, 11:58 a.m. UTC | #1 
Will apply tomorrow if no objections.

Zane

On 12/10/21 9:02 pm, Zane van Iperen wrote:
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>   libavformat/argo_asf.c | 8 ++++----
>   libavformat/argo_asf.h | 3 ++-
>   2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
> index 7e759c7c0c..acf30839b9 100644
> --- a/libavformat/argo_asf.c
> +++ b/libavformat/argo_asf.c
> @@ -52,8 +52,8 @@ void ff_argo_asf_parse_file_header(ArgoASFFileHeader *hdr, const uint8_t *buf)
>       hdr->version_minor  = AV_RL16(buf + 6);
>       hdr->num_chunks     = AV_RL32(buf + 8);
>       hdr->chunk_offset   = AV_RL32(buf + 12);
> -    for (int i = 0; i < FF_ARRAY_ELEMS(hdr->name); i++)
> -        hdr->name[i]    = AV_RL8(buf + 16 + i);
> +    memcpy(hdr->name, buf + 16, ASF_NAME_SIZE);
> +    hdr->name[ASF_NAME_SIZE] = '\0';
>   }
>   
>   int ff_argo_asf_validate_file_header(AVFormatContext *s, const ArgoASFFileHeader *hdr)
> @@ -331,7 +331,7 @@ static void argo_asf_write_file_header(const ArgoASFFileHeader *fhdr, AVIOContex
>       avio_wl16( pb, fhdr->version_minor);
>       avio_wl32( pb, fhdr->num_chunks);
>       avio_wl32( pb, fhdr->chunk_offset);
> -    avio_write(pb, fhdr->name, sizeof(fhdr->name));
> +    avio_write(pb, fhdr->name, ASF_NAME_SIZE);
>   }
>   
>   static void argo_asf_write_chunk_header(const ArgoASFChunkHeader *ckhdr, AVIOContext *pb)
> @@ -368,7 +368,7 @@ static int argo_asf_write_header(AVFormatContext *s)
>       } else {
>           len = end - name;
>       }
> -    memcpy(fhdr.name, name, FFMIN(len, sizeof(fhdr.name)));
> +    memcpy(fhdr.name, name, FFMIN(len, ASF_NAME_SIZE));
>   
>       chdr.num_blocks    = 0;
>       chdr.num_samples   = ASF_SAMPLE_COUNT;
> diff --git a/libavformat/argo_asf.h b/libavformat/argo_asf.h
> index e65125fb79..1fab31a90b 100644
> --- a/libavformat/argo_asf.h
> +++ b/libavformat/argo_asf.h
> @@ -33,6 +33,7 @@
>   #define ASF_CHUNK_HEADER_SIZE   20
>   #define ASF_SAMPLE_COUNT        32
>   #define ASF_MIN_BUFFER_SIZE     FFMAX(ASF_FILE_HEADER_SIZE, ASF_CHUNK_HEADER_SIZE)
> +#define ASF_NAME_SIZE           8
>   
>   typedef struct ArgoASFFileHeader {
>       uint32_t    magic;          /*< Magic Number, {'A', 'S', 'F', '\0'} */
> @@ -40,7 +41,7 @@ typedef struct ArgoASFFileHeader {
>       uint16_t    version_minor;  /*< File Minor Version. */
>       uint32_t    num_chunks;     /*< No. chunks in the file. */
>       uint32_t    chunk_offset;   /*< Offset to the first chunk from the start of the file. */
> -    int8_t      name[8];        /*< Name. */
> +    char        name[ASF_NAME_SIZE + 1]; /*< Name, +1 for NULL-terminator. */
>   } ArgoASFFileHeader;
>   
>   typedef struct ArgoASFChunkHeader {
>
Zhao Zhili Oct. 14, 2021, 12:11 p.m. UTC | #2 
> On Oct 12, 2021, at 7:02 PM, Zane van Iperen <zane@zanevaniperen.com> wrote:
> 
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
> libavformat/argo_asf.c | 8 ++++----
> libavformat/argo_asf.h | 3 ++-
> 2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
> index 7e759c7c0c..acf30839b9 100644
> --- a/libavformat/argo_asf.c
> +++ b/libavformat/argo_asf.c
> @@ -52,8 +52,8 @@ void ff_argo_asf_parse_file_header(ArgoASFFileHeader *hdr, const uint8_t *buf)
>     hdr->version_minor  = AV_RL16(buf + 6);
>     hdr->num_chunks     = AV_RL32(buf + 8);
>     hdr->chunk_offset   = AV_RL32(buf + 12);
> -    for (int i = 0; i < FF_ARRAY_ELEMS(hdr->name); i++)
> -        hdr->name[i]    = AV_RL8(buf + 16 + i);
> +    memcpy(hdr->name, buf + 16, ASF_NAME_SIZE);
> +    hdr->name[ASF_NAME_SIZE] = '\0';
> }
> 
> int ff_argo_asf_validate_file_header(AVFormatContext *s, const ArgoASFFileHeader *hdr)
> @@ -331,7 +331,7 @@ static void argo_asf_write_file_header(const ArgoASFFileHeader *fhdr, AVIOContex
>     avio_wl16( pb, fhdr->version_minor);
>     avio_wl32( pb, fhdr->num_chunks);
>     avio_wl32( pb, fhdr->chunk_offset);
> -    avio_write(pb, fhdr->name, sizeof(fhdr->name));
> +    avio_write(pb, fhdr->name, ASF_NAME_SIZE);
> }
> 
> static void argo_asf_write_chunk_header(const ArgoASFChunkHeader *ckhdr, AVIOContext *pb)
> @@ -368,7 +368,7 @@ static int argo_asf_write_header(AVFormatContext *s)
>     } else {
>         len = end - name;
>     }
> -    memcpy(fhdr.name, name, FFMIN(len, sizeof(fhdr.name)));
> +    memcpy(fhdr.name, name, FFMIN(len, ASF_NAME_SIZE));
> 
>     chdr.num_blocks    = 0;
>     chdr.num_samples   = ASF_SAMPLE_COUNT;
> diff --git a/libavformat/argo_asf.h b/libavformat/argo_asf.h
> index e65125fb79..1fab31a90b 100644
> --- a/libavformat/argo_asf.h
> +++ b/libavformat/argo_asf.h
> @@ -33,6 +33,7 @@
> #define ASF_CHUNK_HEADER_SIZE   20
> #define ASF_SAMPLE_COUNT        32
> #define ASF_MIN_BUFFER_SIZE     FFMAX(ASF_FILE_HEADER_SIZE, ASF_CHUNK_HEADER_SIZE)
> +#define ASF_NAME_SIZE           8
> 
> typedef struct ArgoASFFileHeader {
>     uint32_t    magic;          /*< Magic Number, {'A', 'S', 'F', '\0'} */
> @@ -40,7 +41,7 @@ typedef struct ArgoASFFileHeader {
>     uint16_t    version_minor;  /*< File Minor Version. */
>     uint32_t    num_chunks;     /*< No. chunks in the file. */
>     uint32_t    chunk_offset;   /*< Offset to the first chunk from the start of the file. */
> -    int8_t      name[8];        /*< Name. */
> +    char        name[ASF_NAME_SIZE + 1]; /*< Name, +1 for NULL-terminator. */
> } ArgoASFFileHeader;


I failed to see why null-terminator is needed from the commit message or from the source code.

> 
> typedef struct ArgoASFChunkHeader {
> -- 
> 2.31.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Zane van Iperen Oct. 14, 2021, 12:25 p.m. UTC | #3 
On 14/10/21 10:11 pm, "zhilizhao(赵志立)" wrote:

>> typedef struct ArgoASFFileHeader {
>>      uint32_t    magic;          /*< Magic Number, {'A', 'S', 'F', '\0'} */
>> @@ -40,7 +41,7 @@ typedef struct ArgoASFFileHeader {
>>      uint16_t    version_minor;  /*< File Minor Version. */
>>      uint32_t    num_chunks;     /*< No. chunks in the file. */
>>      uint32_t    chunk_offset;   /*< Offset to the first chunk from the start of the file. */
>> -    int8_t      name[8];        /*< Name. */
>> +    char        name[ASF_NAME_SIZE + 1]; /*< Name, +1 for NULL-terminator. */
>> } ArgoASFFileHeader;
> 
> 
> I failed to see why null-terminator is needed from the commit message or from the source code.
> 

See https://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/286939.html
Zhao Zhili Oct. 14, 2021, 12:45 p.m. UTC | #4 
> On Oct 14, 2021, at 8:25 PM, Zane van Iperen <zane@zanevaniperen.com> wrote:
> 
> 
> 
> On 14/10/21 10:11 pm, "zhilizhao(赵志立)" wrote:
> 
>>> typedef struct ArgoASFFileHeader {
>>>     uint32_t    magic;          /*< Magic Number, {'A', 'S', 'F', '\0'} */
>>> @@ -40,7 +41,7 @@ typedef struct ArgoASFFileHeader {
>>>     uint16_t    version_minor;  /*< File Minor Version. */
>>>     uint32_t    num_chunks;     /*< No. chunks in the file. */
>>>     uint32_t    chunk_offset;   /*< Offset to the first chunk from the start of the file. */
>>> -    int8_t      name[8];        /*< Name. */
>>> +    char        name[ASF_NAME_SIZE + 1]; /*< Name, +1 for NULL-terminator. */
>>> } ArgoASFFileHeader;
>> I failed to see why null-terminator is needed from the commit message or from the source code.
> 
> See https://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/286939.html

Sorry for the noise, email client doesn’t show the patch set together.
Add some description will be helpful when reading the patch alone.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Zane van Iperen Oct. 14, 2021, 1:06 p.m. UTC | #5 
On 14/10/21 10:45 pm, "zhilizhao(赵志立)" wrote:

>>>> } ArgoASFFileHeader;
>>> I failed to see why null-terminator is needed from the commit message or from the source code.
>>
>> See https://ffmpeg.org/pipermail/ffmpeg-devel/2021-October/286939.html
> 
> Sorry for the noise, email client doesn’t show the patch set together.
> Add some description will be helpful when reading the patch alone.
> 

No worries, will do before I push.
diff mbox series

Patch

diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
index 7e759c7c0c..acf30839b9 100644
--- a/libavformat/argo_asf.c
+++ b/libavformat/argo_asf.c
@@ -52,8 +52,8 @@  void ff_argo_asf_parse_file_header(ArgoASFFileHeader *hdr, const uint8_t *buf)
     hdr->version_minor  = AV_RL16(buf + 6);
     hdr->num_chunks     = AV_RL32(buf + 8);
     hdr->chunk_offset   = AV_RL32(buf + 12);
-    for (int i = 0; i < FF_ARRAY_ELEMS(hdr->name); i++)
-        hdr->name[i]    = AV_RL8(buf + 16 + i);
+    memcpy(hdr->name, buf + 16, ASF_NAME_SIZE);
+    hdr->name[ASF_NAME_SIZE] = '\0';
 }
 
 int ff_argo_asf_validate_file_header(AVFormatContext *s, const ArgoASFFileHeader *hdr)
@@ -331,7 +331,7 @@  static void argo_asf_write_file_header(const ArgoASFFileHeader *fhdr, AVIOContex
     avio_wl16( pb, fhdr->version_minor);
     avio_wl32( pb, fhdr->num_chunks);
     avio_wl32( pb, fhdr->chunk_offset);
-    avio_write(pb, fhdr->name, sizeof(fhdr->name));
+    avio_write(pb, fhdr->name, ASF_NAME_SIZE);
 }
 
 static void argo_asf_write_chunk_header(const ArgoASFChunkHeader *ckhdr, AVIOContext *pb)
@@ -368,7 +368,7 @@  static int argo_asf_write_header(AVFormatContext *s)
     } else {
         len = end - name;
     }
-    memcpy(fhdr.name, name, FFMIN(len, sizeof(fhdr.name)));
+    memcpy(fhdr.name, name, FFMIN(len, ASF_NAME_SIZE));
 
     chdr.num_blocks    = 0;
     chdr.num_samples   = ASF_SAMPLE_COUNT;
diff --git a/libavformat/argo_asf.h b/libavformat/argo_asf.h
index e65125fb79..1fab31a90b 100644
--- a/libavformat/argo_asf.h
+++ b/libavformat/argo_asf.h
@@ -33,6 +33,7 @@ 
 #define ASF_CHUNK_HEADER_SIZE   20
 #define ASF_SAMPLE_COUNT        32
 #define ASF_MIN_BUFFER_SIZE     FFMAX(ASF_FILE_HEADER_SIZE, ASF_CHUNK_HEADER_SIZE)
+#define ASF_NAME_SIZE           8
 
 typedef struct ArgoASFFileHeader {
     uint32_t    magic;          /*< Magic Number, {'A', 'S', 'F', '\0'} */
@@ -40,7 +41,7 @@  typedef struct ArgoASFFileHeader {
     uint16_t    version_minor;  /*< File Minor Version. */
     uint32_t    num_chunks;     /*< No. chunks in the file. */
     uint32_t    chunk_offset;   /*< Offset to the first chunk from the start of the file. */
-    int8_t      name[8];        /*< Name. */
+    char        name[ASF_NAME_SIZE + 1]; /*< Name, +1 for NULL-terminator. */
 } ArgoASFFileHeader;
 
 typedef struct ArgoASFChunkHeader {