[FFmpeg-devel] [PATCH 1/4] fftools/textformat: fix writing integers for mermaid

[FFmpeg-devel] [PATCH 1/4] fftools/textformat: fix writing integers for mermaid

[Marvin Scholz]

With the previous logic, the integer writing codepath was unreachable.

Fix CID 1646948
---
 fftools/textformat/tf_mermaid.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/fftools/textformat/tf_mermaid.c b/fftools/textformat/tf_mermaid.c
index d3b9131ada..59b11811f1 100644
--- a/fftools/textformat/tf_mermaid.c
+++ b/fftools/textformat/tf_mermaid.c
@@ -604,22 +604,20 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
 
             break;
         case AV_DIAGRAMTYPE_ENTITYRELATIONSHIP:
-
-            if (!is_int && str)
             {
-                const char *col_type;
+                const char *col_type = "";
 
                 if (key[0] == '_')
                     return;
 
-                if (sec_data.section_id && !strcmp(str, sec_data.section_id))
-                    col_type = "PK";
-                else if (sec_data.dest_id && !strcmp(str, sec_data.dest_id))
-                    col_type = "FK";
-                else if (sec_data.src_id && !strcmp(str, sec_data.src_id))
-                    col_type = "FK";
-                else
-                    col_type = "";
+                if (str) {
+                    if (sec_data.section_id && !strcmp(str, sec_data.section_id))
+                        col_type = "PK";
+                    else if (sec_data.dest_id && !strcmp(str, sec_data.dest_id))
+                        col_type = "FK";
+                    else if (sec_data.src_id && !strcmp(str, sec_data.src_id))
+                        col_type = "FK";
+                }
 
                 MM_INDENT();
 
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/4] fftools/textformat: exit early in mermaid_print_value

[Marvin Scholz]

Doesn't change the logic, instead of exiting in each of the two
branches below, just exit before.
---
 fftools/textformat/tf_mermaid.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/fftools/textformat/tf_mermaid.c b/fftools/textformat/tf_mermaid.c
index 59b11811f1..dbe489a7a7 100644
--- a/fftools/textformat/tf_mermaid.c
+++ b/fftools/textformat/tf_mermaid.c
@@ -579,15 +579,11 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
         exit = 1;
     }
 
-    //if (exit)
-    //    return;
+    if (exit)
+        return;
 
     if ((section->flags & (AV_TEXTFORMAT_SECTION_FLAG_IS_SHAPE | AV_TEXTFORMAT_SECTION_PRINT_TAGS))
         || (section->flags & AV_TEXTFORMAT_SECTION_FLAG_IS_SUBGRAPH && sec_data.subgraph_start_incomplete)) {
-
-        if (exit)
-            return;
-
         switch (mmc->diagram_config->diagram_type) {
         case AV_DIAGRAMTYPE_GRAPH:
 
@@ -630,10 +626,6 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
         }
 
     } else if (section->flags & AV_TEXTFORMAT_SECTION_FLAG_HAS_LINKS) {
-
-        if (exit)
-            return;
-
         if (buf->len > 0)
             av_bprintf(buf, "%s", "<br>");
 
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in mermaid_print_value

[Marvin Scholz]

Instead of the caller having to indicate if it is passing an
integer, just use the fact that str is NULL when an integer is
to be printed.
---
 fftools/textformat/tf_mermaid.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fftools/textformat/tf_mermaid.c b/fftools/textformat/tf_mermaid.c
index dbe489a7a7..da371c8fff 100644
--- a/fftools/textformat/tf_mermaid.c
+++ b/fftools/textformat/tf_mermaid.c
@@ -547,7 +547,7 @@ static void mermaid_print_section_footer(AVTextFormatContext *tfc)
 }
 
 static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
-                                const char *str, int64_t num, const int is_int)
+                                const char *str, int64_t num)
 {
     MermaidContext *mmc = tfc->priv;
     const AVTextFormatSection *section = tf_get_section(tfc, tfc->level);
@@ -587,7 +587,7 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
         switch (mmc->diagram_config->diagram_type) {
         case AV_DIAGRAMTYPE_GRAPH:
 
-            if (is_int) {
+            if (!str) {
                 writer_printf(tfc, "<span class=\"%s\">%s: %"PRId64"</span>", key, key, num);
             } else {
                 ////AVBPrint b;
@@ -617,7 +617,7 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
 
                 MM_INDENT();
 
-                if (is_int)
+                if (!str)
                     writer_printf(tfc, "    %s %"PRId64" %s\n", key, num, col_type);
                 else
                     writer_printf(tfc, "    %s %s %s\n", key, str, col_type);
@@ -630,7 +630,7 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
             av_bprintf(buf, "%s", "<br>");
 
         av_bprintf(buf, "");
-        if (is_int)
+        if (!str)
             av_bprintf(buf, "<span>%s: %"PRId64"</span>", key, num);
         else
             av_bprintf(buf, "<span>%s</span>", str);
@@ -641,12 +641,12 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
 
 static inline void mermaid_print_str(AVTextFormatContext *tfc, const char *key, const char *value)
 {
-    mermaid_print_value(tfc, key, value, 0, 0);
+    mermaid_print_value(tfc, key, value, 0);
 }
 
 static void mermaid_print_int(AVTextFormatContext *tfc, const char *key, int64_t value)
 {
-    mermaid_print_value(tfc, key, NULL, value, 1);
+    mermaid_print_value(tfc, key, NULL, value);
 }
 
 const AVTextFormatter avtextformatter_mermaid = {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/4] fftools/textformat: remove leftover comments in mermaid_print_value

[Marvin Scholz]

Remove some leftover commented code and an extraneous semicolon.
---
 fftools/textformat/tf_mermaid.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fftools/textformat/tf_mermaid.c b/fftools/textformat/tf_mermaid.c
index da371c8fff..9d98b6a36f 100644
--- a/fftools/textformat/tf_mermaid.c
+++ b/fftools/textformat/tf_mermaid.c
@@ -575,7 +575,7 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
     }
 
     if (section->linktype_key && !strcmp(section->linktype_key, key)) {
-        mmc->section_data[tfc->level].link_type = (AVTextFormatLinkType)num;;
+        mmc->section_data[tfc->level].link_type = (AVTextFormatLinkType)num;
         exit = 1;
     }
 
@@ -590,10 +590,7 @@ static void mermaid_print_value(AVTextFormatContext *tfc, const char *key,
             if (!str) {
                 writer_printf(tfc, "<span class=\"%s\">%s: %"PRId64"</span>", key, key, num);
             } else {
-                ////AVBPrint b;
-                ////av_bprint_init(&b, 0, AV_BPRINT_SIZE_UNLIMITED);
                 const char *tmp = av_strireplace(str, "\"", "'");
-                ////av_bprint_escape(&b, str, NULL, AV_ESCAPE_MODE_AUTO, AV_ESCAPE_FLAG_STRICT);
                 writer_printf(tfc, "<span class=\"%s\">%s</span>", key, tmp);
                 av_freep(&tmp);
             }
-- 
2.39.5 (Apple Git-154)

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in mermaid_print_value

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marvin Scholz
> Sent: Mittwoch, 11. Juni 2025 21:57
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in
> mermaid_print_value
> 
> Instead of the caller having to indicate if it is passing an
> integer, just use the fact that str is NULL when an integer is
> to be printed.
> ---

Hi Marvin,

thanks a lot for the patches, I'll review them in detail shortly.

Just for this specific case: the reason for having the is_int
parameter was about avoiding to print a zero in cases when the string 
is NULL.

Thanks,
sw
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/4] fftools/textformat: fix writing integers for mermaid

[softworkz .]

Hi Marvin,

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marvin Scholz
> Sent: Mittwoch, 11. Juni 2025 21:57
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 1/4] fftools/textformat: fix writing
> integers for mermaid
> 
> With the previous logic, the integer writing codepath was
> unreachable.
> 
> Fix CID 1646948
> ---
>  fftools/textformat/tf_mermaid.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/fftools/textformat/tf_mermaid.c
> b/fftools/textformat/tf_mermaid.c
> index d3b9131ada..59b11811f1 100644
> --- a/fftools/textformat/tf_mermaid.c
> +++ b/fftools/textformat/tf_mermaid.c
> @@ -604,22 +604,20 @@ static void
> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
> 
>              break;
>          case AV_DIAGRAMTYPE_ENTITYRELATIONSHIP:
> -
> -            if (!is_int && str)
>              {

This is actually intended. The reason is that Mermaid ER diagrams do not allow
number values for column and data type names while for AV_DIAGRAMTYPE_GRAPH,
it's not a problem. Also, column names must not be NULL.


Still, you are right that there's unreachable code, which is the first branch
of this if-block.

  if (is_int)
      writer_printf(tfc, "    %s %"PRId64" %s\n", key, num, col_type);
  else
      writer_printf(tfc, "    %s %s %s\n", key, str, col_type);


That first part can be removed, only the 2nd is needed.


Thanks
sw

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/4] fftools/textformat: exit early in mermaid_print_value

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marvin Scholz
> Sent: Mittwoch, 11. Juni 2025 21:57
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 2/4] fftools/textformat: exit early in
> mermaid_print_value
> 
> Doesn't change the logic, instead of exiting in each of the two
> branches below, just exit before.
> ---
>  fftools/textformat/tf_mermaid.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/fftools/textformat/tf_mermaid.c
> b/fftools/textformat/tf_mermaid.c
> index 59b11811f1..dbe489a7a7 100644
> --- a/fftools/textformat/tf_mermaid.c
> +++ b/fftools/textformat/tf_mermaid.c
> @@ -579,15 +579,11 @@ static void
> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
>          exit = 1;
>      }
> 
> -    //if (exit)
> -    //    return;
> +    if (exit)
> +        return;
> 
>      if ((section->flags & (AV_TEXTFORMAT_SECTION_FLAG_IS_SHAPE |
> AV_TEXTFORMAT_SECTION_PRINT_TAGS))
>          || (section->flags & AV_TEXTFORMAT_SECTION_FLAG_IS_SUBGRAPH
> && sec_data.subgraph_start_incomplete)) {
> -
> -        if (exit)
> -            return;
> -
>          switch (mmc->diagram_config->diagram_type) {
>          case AV_DIAGRAMTYPE_GRAPH:
> 
> @@ -630,10 +626,6 @@ static void
> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
>          }
> 
>      } else if (section->flags &
> AV_TEXTFORMAT_SECTION_FLAG_HAS_LINKS) {
> -
> -        if (exit)
> -            return;
> -
>          if (buf->len > 0)
>              av_bprintf(buf, "%s", "<br>");
> 
> --
> 2.39.5 (Apple Git-154)
> 
> _______________________________________________

LGTM, thanks!

sw




_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in mermaid_print_value

[softworkz .]

Hi Marvin,

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marvin Scholz
> Sent: Mittwoch, 11. Juni 2025 21:57
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in
> mermaid_print_value
> 
> Instead of the caller having to indicate if it is passing an
> integer, just use the fact that str is NULL when an integer is
> to be printed.
> ---
>  fftools/textformat/tf_mermaid.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fftools/textformat/tf_mermaid.c
> b/fftools/textformat/tf_mermaid.c
> index dbe489a7a7..da371c8fff 100644
> --- a/fftools/textformat/tf_mermaid.c
> +++ b/fftools/textformat/tf_mermaid.c
> @@ -547,7 +547,7 @@ static void
> mermaid_print_section_footer(AVTextFormatContext *tfc)
>  }
> 
>  static void mermaid_print_value(AVTextFormatContext *tfc, const char
> *key,
> -                                const char *str, int64_t num, const
> int is_int)
> +                                const char *str, int64_t num)
>  {
>      MermaidContext *mmc = tfc->priv;
>      const AVTextFormatSection *section = tf_get_section(tfc, tfc-
> >level);
> @@ -587,7 +587,7 @@ static void
> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
>          switch (mmc->diagram_config->diagram_type) {
>          case AV_DIAGRAMTYPE_GRAPH:
> 
> -            if (is_int) {
> +            if (!str) {

This is not right - it would print 0 when the str is NULL.
See tf_xml, it uses the same pattern (is_int).

Thanks,
sw

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] fftools/textformat: remove leftover comments in mermaid_print_value

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marvin Scholz
> Sent: Mittwoch, 11. Juni 2025 21:57
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 4/4] fftools/textformat: remove
> leftover comments in mermaid_print_value
> 
> Remove some leftover commented code and an extraneous semicolon.
> ---
>  fftools/textformat/tf_mermaid.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fftools/textformat/tf_mermaid.c
> b/fftools/textformat/tf_mermaid.c
> index da371c8fff..9d98b6a36f 100644
> --- a/fftools/textformat/tf_mermaid.c
> +++ b/fftools/textformat/tf_mermaid.c
> @@ -575,7 +575,7 @@ static void
> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
>      }
> 
>      if (section->linktype_key && !strcmp(section->linktype_key,
> key)) {
> -        mmc->section_data[tfc->level].link_type =
> (AVTextFormatLinkType)num;;
> +        mmc->section_data[tfc->level].link_type =
> (AVTextFormatLinkType)num;
>          exit = 1;
>      }
> 
> @@ -590,10 +590,7 @@ static void
> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
>              if (!str) {
>                  writer_printf(tfc, "<span class=\"%s\">%s:
> %"PRId64"</span>", key, key, num);
>              } else {
> -                ////AVBPrint b;
> -                ////av_bprint_init(&b, 0, AV_BPRINT_SIZE_UNLIMITED);
>                  const char *tmp = av_strireplace(str, "\"", "'");
> -                ////av_bprint_escape(&b, str, NULL,
> AV_ESCAPE_MODE_AUTO, AV_ESCAPE_FLAG_STRICT);
>                  writer_printf(tfc, "<span class=\"%s\">%s</span>",
> key, tmp);
>                  av_freep(&tmp);
>              }
> --
> 2.39.5 (Apple Git-154)
> 
> _______________________________________________

LGTM, thanks!

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in mermaid_print_value

[Marvin Scholz]

On 12 Jun 2025, at 4:19, softworkz . wrote:

> Hi Marvin,
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Marvin Scholz
>> Sent: Mittwoch, 11. Juni 2025 21:57
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in
>> mermaid_print_value
>>
>> Instead of the caller having to indicate if it is passing an
>> integer, just use the fact that str is NULL when an integer is
>> to be printed.
>> ---
>>  fftools/textformat/tf_mermaid.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fftools/textformat/tf_mermaid.c
>> b/fftools/textformat/tf_mermaid.c
>> index dbe489a7a7..da371c8fff 100644
>> --- a/fftools/textformat/tf_mermaid.c
>> +++ b/fftools/textformat/tf_mermaid.c
>> @@ -547,7 +547,7 @@ static void
>> mermaid_print_section_footer(AVTextFormatContext *tfc)
>>  }
>>
>>  static void mermaid_print_value(AVTextFormatContext *tfc, const char
>> *key,
>> -                                const char *str, int64_t num, const
>> int is_int)
>> +                                const char *str, int64_t num)
>>  {
>>      MermaidContext *mmc = tfc->priv;
>>      const AVTextFormatSection *section = tf_get_section(tfc, tfc-
>>> level);
>> @@ -587,7 +587,7 @@ static void
>> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
>>          switch (mmc->diagram_config->diagram_type) {
>>          case AV_DIAGRAMTYPE_GRAPH:
>>
>> -            if (is_int) {
>> +            if (!str) {
>
> This is not right - it would print 0 when the str is NULL.
> See tf_xml, it uses the same pattern (is_int).
>

So what do you expect to happen when it is NULL?

From what I have seen passing NULL for str would
make these functions actually UB as passing NULL
for a string to printf like functions is not defined,
unless I missed this being handled explicitly somewhere.

> Thanks,
> sw
>
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer type in mermaid_print_value

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marvin Scholz
> Sent: Donnerstag, 12. Juni 2025 13:34
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer
> type in mermaid_print_value
> 
> 
> 
> On 12 Jun 2025, at 4:19, softworkz . wrote:
> 
> > Hi Marvin,
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> Of
> >> Marvin Scholz
> >> Sent: Mittwoch, 11. Juni 2025 21:57
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: [FFmpeg-devel] [PATCH 3/4] fftools/textformat: infer
> type in
> >> mermaid_print_value
> >>
> >> Instead of the caller having to indicate if it is passing an
> >> integer, just use the fact that str is NULL when an integer is
> >> to be printed.
> >> ---
> >>  fftools/textformat/tf_mermaid.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/fftools/textformat/tf_mermaid.c
> >> b/fftools/textformat/tf_mermaid.c
> >> index dbe489a7a7..da371c8fff 100644
> >> --- a/fftools/textformat/tf_mermaid.c
> >> +++ b/fftools/textformat/tf_mermaid.c
> >> @@ -547,7 +547,7 @@ static void
> >> mermaid_print_section_footer(AVTextFormatContext *tfc)
> >>  }
> >>
> >>  static void mermaid_print_value(AVTextFormatContext *tfc, const
> char
> >> *key,
> >> -                                const char *str, int64_t num,
> const
> >> int is_int)
> >> +                                const char *str, int64_t num)
> >>  {
> >>      MermaidContext *mmc = tfc->priv;
> >>      const AVTextFormatSection *section = tf_get_section(tfc,
> tfc-
> >>> level);
> >> @@ -587,7 +587,7 @@ static void
> >> mermaid_print_value(AVTextFormatContext *tfc, const char *key,
> >>          switch (mmc->diagram_config->diagram_type) {
> >>          case AV_DIAGRAMTYPE_GRAPH:
> >>
> >> -            if (is_int) {
> >> +            if (!str) {
> >
> > This is not right - it would print 0 when the str is NULL.
> > See tf_xml, it uses the same pattern (is_int).
> >
> 
> So what do you expect to happen when it is NULL?
> 
> From what I have seen passing NULL for str would
> make these functions actually UB as passing NULL
> for a string to printf like functions is not defined,
> unless I missed this being handled explicitly somewhere.

Hi Marvin,

you are surely right to point at this. There is still an ongoing
discussion about parameter validation (with Stefano Sabatini) and 
I have a patchset for it that isn't submitted yet. This adds 
parameter validation in several ways and to several functions.
For example, for the corresponding function in tf_xml.c, it 
adds this to the function at the start:

static void xml_print_value(AVTextFormatContext *wctx, const char *key,
                            const char *str, int64_t num, const int is_int)
{
    AVBPrint buf;
    XMLContext *xml = wctx->priv;
    const struct AVTextFormatSection *section = tf_get_section(wctx, wctx->level);
    
    if (!section)
        return;
    
    if (!tf_validate_string_param(key, "key", wctx) ||
        (!is_int && !tf_validate_string_param(str, "value", wctx))) {
        return;
    }
....

This kind of "plot hole" has existed ever since in the original code.
The merged patchset was about generalizing and refactoring code but 
not about making functional changes to the APIs. I made a cut at this
point to avoid the patchset to become even more excessive. 

I will submit the next step about parameter validation in a while,
when I find some time to revisit this topic.

Thanks,
sw

_______________________________________________
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".