unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Warning in svg_load_image
       [not found] <87ilt8hcz4.fsf.ref@yahoo.com>
@ 2022-02-21  7:53 ` Po Lu
  2022-02-21 13:26   ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Po Lu @ 2022-02-21  7:53 UTC (permalink / raw)
  To: emacs-devel

I get this building on i586-pc-solaris2.11 with rsvg support:

image.c: In function 'svg_load_image':
image.c:10776:7: warning: '%f' directive output may be truncated writing between 8 and 317 bytes into a region of size between 167 and 187 [-Wformat-truncation=]
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
image.c:10780:22: note: format string is defined here
       "viewBox=\"0 0 %f %f\">"
                      ^~
image.c:10776:7: note: directive argument in the range [0, 16777215]
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
image.c:10776:7: note: assuming directive output of 1 byte
image.c:10802:24: note: 'snprintf' output 330 or more bytes (assuming 331) into a destination of size 383
     if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      foreground & 0xFFFFFF, width, height,
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      viewbox_width, viewbox_height,
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      background & 0xFFFFFF,
      ~~~~~~~~~~~~~~~~~~~~~~
      SSDATA (encoded_contents)))
      ~~~~~~~~~~~~~~~~~~~~~~~~~~

Does anyone want to fix this?

Thanks.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-21  7:53 ` Warning in svg_load_image Po Lu
@ 2022-02-21 13:26   ` Eli Zaretskii
  2022-02-21 13:37     ` Po Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2022-02-21 13:26 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Date: Mon, 21 Feb 2022 15:53:51 +0800
> 
> image.c: In function 'svg_load_image':
> image.c:10776:7: warning: '%f' directive output may be truncated writing between 8 and 317 bytes into a region of size between 167 and 187 [-Wformat-truncation=]
>        "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> image.c:10780:22: note: format string is defined here
>        "viewBox=\"0 0 %f %f\">"
>                       ^~
> image.c:10776:7: note: directive argument in the range [0, 16777215]
>        "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> image.c:10776:7: note: assuming directive output of 1 byte
> image.c:10802:24: note: 'snprintf' output 330 or more bytes (assuming 331) into a destination of size 383
>      if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       foreground & 0xFFFFFF, width, height,
>       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       viewbox_width, viewbox_height,
>       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>       background & 0xFFFFFF,
>       ~~~~~~~~~~~~~~~~~~~~~~
>       SSDATA (encoded_contents)))
>       ~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Does anyone want to fix this?

Not really, but does the below fix these, by chance?

diff --git a/src/image.c b/src/image.c
index e2ba744..02b58b9 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10632,9 +10632,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       strncpy (css, SSDATA (lcss), SBYTES (lcss));
       *(css + SBYTES (lcss) + 1) = 0;
     }
-#endif
+#endif	/* LIBRSVG >= 2.48.0 */
 
-#else
+#else  /* LIBRSVG < 2.32.0 */
   /* Make a handle to a new rsvg object.  */
   rsvg_handle = rsvg_handle_new ();
   eassume (rsvg_handle);
@@ -10657,7 +10657,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
      it for further writes.  */
   rsvg_handle_close (rsvg_handle, &err);
   if (err) goto rsvg_error;
-#endif
+#endif	/* LIBRSVG 2.32.0 */
 
   /* Get the image dimensions.  */
 #if LIBRSVG_CHECK_VERSION (2, 46, 0)
@@ -10727,13 +10727,13 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 	  viewbox_height = viewbox.y + viewbox.height;
 	}
     }
-#else
+#else  /* LIBRSVG < 2.46.0 */
   /* In librsvg before 2.46.0, guess the viewbox from the image dimensions.  */
   RsvgDimensionData dimension_data;
   rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
   viewbox_width = dimension_data.width;
   viewbox_height = dimension_data.height;
-#endif
+#endif	/* LIBRSVG < 2.46.0 */
 
 #ifdef HAVE_NATIVE_TRANSFORMS
   compute_image_size (viewbox_width, viewbox_height, img,
@@ -10777,7 +10777,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
       "style=\"color: #%06X; fill: currentColor;\" "
       "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
-      "viewBox=\"0 0 %f %f\">"
+      "viewBox=\"0 0 %.0f %.0f\">"
       "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
       "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
       "</svg>";
@@ -10801,7 +10801,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 
     if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
 				 foreground & 0xFFFFFF, width, height,
-				 viewbox_width, viewbox_height,
+				 /* Sanitize the viewBox dimensions.  */
+				 min (viewbox_width, 10000.),
+				 min (viewbox_height, 10000.),
 				 background & 0xFFFFFF,
 				 SSDATA (encoded_contents)))
       goto rsvg_error;



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-21 13:26   ` Eli Zaretskii
@ 2022-02-21 13:37     ` Po Lu
  2022-02-21 13:51       ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Po Lu @ 2022-02-21 13:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Not really, but does the below fix these, by chance?

Unfortunately not.  Now I get this slightly different warning:

  CC       image.o
image.c: In function 'svg_load_image':
image.c:10776:7: warning: '%.0f' directive output may be truncated writing between 1 and 310 bytes into a region of size between 171 and 191 [-Wformat-truncation=]
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
image.c:10780:22: note: format string is defined here
       "viewBox=\"0 0 %.0f %.0f\">"
                      ^~~~
image.c:10776:7: note: directive argument in the range [0, 16777215]
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
image.c:10776:7: note: assuming directive output of 1 byte
image.c:10802:24: note: 'snprintf' output 316 or more bytes (assuming 317) into a destination of size 387
     if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      foreground & 0xFFFFFF, width, height,
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /* Sanitize the viewBox dimensions.  */
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      min (viewbox_width, 10000.),
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      min (viewbox_height, 10000.),
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      background & 0xFFFFFF,
      ~~~~~~~~~~~~~~~~~~~~~~
      SSDATA (encoded_contents)))
      ~~~~~~~~~~~~~~~~~~~~~~~~~~

Thanks.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-21 13:37     ` Po Lu
@ 2022-02-21 13:51       ` Eli Zaretskii
  2022-02-22  3:53         ` Po Lu
                           ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Eli Zaretskii @ 2022-02-21 13:51 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Mon, 21 Feb 2022 21:37:44 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Not really, but does the below fix these, by chance?
> 
> Unfortunately not.  Now I get this slightly different warning:

What about the one below:

diff --git a/src/image.c b/src/image.c
index e2ba744..fca5236 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10632,9 +10632,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       strncpy (css, SSDATA (lcss), SBYTES (lcss));
       *(css + SBYTES (lcss) + 1) = 0;
     }
-#endif
+#endif	/* LIBRSVG >= 2.48.0 */
 
-#else
+#else  /* LIBRSVG < 2.32.0 */
   /* Make a handle to a new rsvg object.  */
   rsvg_handle = rsvg_handle_new ();
   eassume (rsvg_handle);
@@ -10657,7 +10657,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
      it for further writes.  */
   rsvg_handle_close (rsvg_handle, &err);
   if (err) goto rsvg_error;
-#endif
+#endif	/* LIBRSVG 2.32.0 */
 
   /* Get the image dimensions.  */
 #if LIBRSVG_CHECK_VERSION (2, 46, 0)
@@ -10727,13 +10727,13 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 	  viewbox_height = viewbox.y + viewbox.height;
 	}
     }
-#else
+#else  /* LIBRSVG < 2.46.0 */
   /* In librsvg before 2.46.0, guess the viewbox from the image dimensions.  */
   RsvgDimensionData dimension_data;
   rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
   viewbox_width = dimension_data.width;
   viewbox_height = dimension_data.height;
-#endif
+#endif	/* LIBRSVG < 2.46.0 */
 
 #ifdef HAVE_NATIVE_TRANSFORMS
   compute_image_size (viewbox_width, viewbox_height, img,
@@ -10777,7 +10777,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
       "style=\"color: #%06X; fill: currentColor;\" "
       "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
-      "viewBox=\"0 0 %f %f\">"
+      "viewBox=\"0 0 %5.0f %5.0f\">"
       "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
       "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
       "</svg>";
@@ -10801,7 +10801,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 
     if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
 				 foreground & 0xFFFFFF, width, height,
-				 viewbox_width, viewbox_height,
+				 /* Sanitize the viewBox dimensions.  */
+				 min (viewbox_width, 10000.),
+				 min (viewbox_height, 10000.),
 				 background & 0xFFFFFF,
 				 SSDATA (encoded_contents)))
       goto rsvg_error;



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-21 13:51       ` Eli Zaretskii
@ 2022-02-22  3:53         ` Po Lu
  2022-02-22 12:32           ` Eli Zaretskii
  2022-02-23 17:11         ` Michael Welsh Duggan
       [not found]         ` <87pmndmrsz.fsf@md5i.com>
  2 siblings, 1 reply; 15+ messages in thread
From: Po Lu @ 2022-02-22  3:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> What about the one below:

That also gives another warning:

image.c: In function 'svg_load_image':
image.c:10776:7: warning: '%5.0f' directive output may be truncated writing between 5 and 310 bytes into a region of size between 173 and 193 [-Wformat-truncation=]
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
image.c:10780:22: note: format string is defined here
       "viewBox=\"0 0 %5.0f %5.0f\">"
                      ^~~~~
image.c:10776:7: note: directive argument in the range [0, 16777215]
       "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\" "
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
image.c:10776:7: note: assuming directive output of 1 byte
image.c:10802:24: note: 'snprintf' output 324 or more bytes (assuming 325) into a destination of size 389
     if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      foreground & 0xFFFFFF, width, height,
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /* Sanitize the viewBox dimensions.  */
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      min (viewbox_width, 10000.),
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      min (viewbox_height, 10000.),
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      background & 0xFFFFFF,
      ~~~~~~~~~~~~~~~~~~~~~~
      SSDATA (encoded_contents)))

(I don't know my way around SVG, so I can't fix it myself, sorry.)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-22  3:53         ` Po Lu
@ 2022-02-22 12:32           ` Eli Zaretskii
  2022-02-22 12:45             ` Andreas Schwab
  2022-02-22 13:02             ` Po Lu
  0 siblings, 2 replies; 15+ messages in thread
From: Eli Zaretskii @ 2022-02-22 12:32 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Feb 2022 11:53:07 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What about the one below:
> 
> That also gives another warning:

Sigh.  Last chance:

diff --git a/src/image.c b/src/image.c
index e2ba744..eb16a3f 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10632,9 +10632,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       strncpy (css, SSDATA (lcss), SBYTES (lcss));
       *(css + SBYTES (lcss) + 1) = 0;
     }
-#endif
+#endif	/* LIBRSVG >= 2.48.0 */
 
-#else
+#else  /* LIBRSVG < 2.32.0 */
   /* Make a handle to a new rsvg object.  */
   rsvg_handle = rsvg_handle_new ();
   eassume (rsvg_handle);
@@ -10657,7 +10657,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
      it for further writes.  */
   rsvg_handle_close (rsvg_handle, &err);
   if (err) goto rsvg_error;
-#endif
+#endif	/* LIBRSVG 2.32.0 */
 
   /* Get the image dimensions.  */
 #if LIBRSVG_CHECK_VERSION (2, 46, 0)
@@ -10727,13 +10727,13 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 	  viewbox_height = viewbox.y + viewbox.height;
 	}
     }
-#else
+#else  /* LIBRSVG < 2.46.0 */
   /* In librsvg before 2.46.0, guess the viewbox from the image dimensions.  */
   RsvgDimensionData dimension_data;
   rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
   viewbox_width = dimension_data.width;
   viewbox_height = dimension_data.height;
-#endif
+#endif	/* LIBRSVG < 2.46.0 */
 
 #ifdef HAVE_NATIVE_TRANSFORMS
   compute_image_size (viewbox_width, viewbox_height, img,
@@ -10777,7 +10777,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
       "style=\"color: #%06X; fill: currentColor;\" "
       "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
-      "viewBox=\"0 0 %f %f\">"
+      "viewBox=\"0 0 %5.0f %5.0f\">"
       "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
       "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
       "</svg>";
@@ -10801,7 +10801,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 
     if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
 				 foreground & 0xFFFFFF, width, height,
-				 viewbox_width, viewbox_height,
+				 /* Sanitize the viewBox dimensions.  */
+				 min (max (viewbox_width, 1.), 10000.),
+				 min (max (viewbox_height, 1.), 10000.),
 				 background & 0xFFFFFF,
 				 SSDATA (encoded_contents)))
       goto rsvg_error;



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-22 12:32           ` Eli Zaretskii
@ 2022-02-22 12:45             ` Andreas Schwab
  2022-02-22 13:02             ` Po Lu
  1 sibling, 0 replies; 15+ messages in thread
From: Andreas Schwab @ 2022-02-22 12:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, emacs-devel

On Feb 22 2022, Eli Zaretskii wrote:

> @@ -10777,7 +10777,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
>        "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
>        "style=\"color: #%06X; fill: currentColor;\" "
>        "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
> -      "viewBox=\"0 0 %f %f\">"
> +      "viewBox=\"0 0 %5.0f %5.0f\">"

I don't think the format width makes a difference, apart from adding
redundant spaces.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-22 12:32           ` Eli Zaretskii
  2022-02-22 12:45             ` Andreas Schwab
@ 2022-02-22 13:02             ` Po Lu
  1 sibling, 0 replies; 15+ messages in thread
From: Po Lu @ 2022-02-22 13:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Sigh.  Last chance:

Didn't help, sadly.  I get another similar error.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-21 13:51       ` Eli Zaretskii
  2022-02-22  3:53         ` Po Lu
@ 2022-02-23 17:11         ` Michael Welsh Duggan
  2022-02-23 17:37           ` Eli Zaretskii
       [not found]         ` <87pmndmrsz.fsf@md5i.com>
  2 siblings, 1 reply; 15+ messages in thread
From: Michael Welsh Duggan @ 2022-02-23 17:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>        "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
>        "style=\"color: #%06X; fill: currentColor;\" "
>        "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
> -      "viewBox=\"0 0 %f %f\">"
> +      "viewBox=\"0 0 %5.0f %5.0f\">"
>        "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
>        "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
>        "</svg>";
> @@ -10801,7 +10801,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
>  
>      if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
>  				 foreground & 0xFFFFFF, width, height,
> -				 viewbox_width, viewbox_height,
> +				 /* Sanitize the viewBox dimensions.  */
> +				 min (viewbox_width, 10000.),
> +				 min (viewbox_height, 10000.),
>  				 background & 0xFFFFFF,
>  				 SSDATA (encoded_contents)))
>        goto rsvg_error;
>

So, a couple of questions and comments...  

As mentioned in other messages, % sizes affect only the minimum sizes of
results, so changing those values shouldn't help.  (It's possible that
using * might, but only as a possible subversion of the heuristics that
this warning uses.)

The principled way to solve this would be to call the snprintf twice,
the first time with a zero-sized buffer, and then to use the return
value to allocate the actual buffer.  This is a pessimisation, but I
don't know if it's a bad one (it depends on how frequently this code
would be called.

Po Lu, what architecture are you compiling for?  As I haven't been able
to trigger this in my own builds, I am guessing that the fact that you
are is due to differences in how the compiler is interpreting the
possible range of values for %f and maybe for %d.  What are sizeof(int)
and sizeof(float) on this architecture?

-- 
Michael Welsh Duggan
(md5i@md5i.com)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-23 17:11         ` Michael Welsh Duggan
@ 2022-02-23 17:37           ` Eli Zaretskii
  2022-02-23 21:58             ` Michael Welsh Duggan
  2022-02-23 22:55             ` Andreas Schwab
  0 siblings, 2 replies; 15+ messages in thread
From: Eli Zaretskii @ 2022-02-23 17:37 UTC (permalink / raw)
  To: Michael Welsh Duggan; +Cc: luangruo, emacs-devel

> From: Michael Welsh Duggan <mwd@md5i.com>
> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
> Date: Wed, 23 Feb 2022 12:11:24 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >        "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
> >        "style=\"color: #%06X; fill: currentColor;\" "
> >        "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
> > -      "viewBox=\"0 0 %f %f\">"
> > +      "viewBox=\"0 0 %5.0f %5.0f\">"
> >        "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
> >        "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
> >        "</svg>";
> > @@ -10801,7 +10801,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
> >  
> >      if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
> >  				 foreground & 0xFFFFFF, width, height,
> > -				 viewbox_width, viewbox_height,
> > +				 /* Sanitize the viewBox dimensions.  */
> > +				 min (viewbox_width, 10000.),
> > +				 min (viewbox_height, 10000.),
> >  				 background & 0xFFFFFF,
> >  				 SSDATA (encoded_contents)))
> >        goto rsvg_error;
> >
> 
> So, a couple of questions and comments...  
> 
> As mentioned in other messages, % sizes affect only the minimum sizes of
> results, so changing those values shouldn't help.  (It's possible that
> using * might, but only as a possible subversion of the heuristics that
> this warning uses.)

If the compiler doesn't understand that the value is being limited to
a maximum of 5 digits, then it shouldn't attempt to emit such
"helpful" warnings.

> The principled way to solve this would be to call the snprintf twice,
> the first time with a zero-sized buffer, and then to use the return
> value to allocate the actual buffer.  This is a pessimisation, but I
> don't know if it's a bad one (it depends on how frequently this code
> would be called.

This is madness.  I'd rather we used a pragma to disable that
particular warning around this part of the code than jump through
hoops because the compiler is too stupid to understand the code it
warns about.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-23 17:37           ` Eli Zaretskii
@ 2022-02-23 21:58             ` Michael Welsh Duggan
  2022-02-24  6:47               ` Eli Zaretskii
  2022-02-23 22:55             ` Andreas Schwab
  1 sibling, 1 reply; 15+ messages in thread
From: Michael Welsh Duggan @ 2022-02-23 21:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Michael Welsh Duggan, luangruo, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Michael Welsh Duggan <mwd@md5i.com>
>> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
>> Date: Wed, 23 Feb 2022 12:11:24 -0500
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >        "xmlns:xi=\"http://www.w3.org/2001/XInclude\" "
>> >        "style=\"color: #%06X; fill: currentColor;\" "
>> >        "width=\"%d\" height=\"%d\" preserveAspectRatio=\"none\" "
>> > -      "viewBox=\"0 0 %f %f\">"
>> > +      "viewBox=\"0 0 %5.0f %5.0f\">"
>> >        "<rect width=\"100%%\" height=\"100%%\" fill=\"#%06X\"/>"
>> >        "<xi:include href=\"data:image/svg+xml;base64,%s\"></xi:include>"
>> >        "</svg>";
>> > @@ -10801,7 +10801,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
>> >  
>> >      if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
>> >  				 foreground & 0xFFFFFF, width, height,
>> > -				 viewbox_width, viewbox_height,
>> > +				 /* Sanitize the viewBox dimensions.  */
>> > +				 min (viewbox_width, 10000.),
>> > +				 min (viewbox_height, 10000.),
>> >  				 background & 0xFFFFFF,
>> >  				 SSDATA (encoded_contents)))
>> >        goto rsvg_error;
>> >
>> 
>> So, a couple of questions and comments...  
>> 
>> As mentioned in other messages, % sizes affect only the minimum sizes of
>> results, so changing those values shouldn't help.  (It's possible that
>> using * might, but only as a possible subversion of the heuristics that
>> this warning uses.)
>
> If the compiler doesn't understand that the value is being limited to
> a maximum of 5 digits, then it shouldn't attempt to emit such
> "helpful" warnings.

Is it being limited?  What is limiting it?  "%5.0f" will not limit it's
size; it will only limit its minimum size, unless I am misunderstanding
the printf specs.

>> The principled way to solve this would be to call the snprintf twice,
>> the first time with a zero-sized buffer, and then to use the return
>> value to allocate the actual buffer.  This is a pessimisation, but I
>> don't know if it's a bad one (it depends on how frequently this code
>> would be called.
>
> This is madness.  I'd rather we used a pragma to disable that
> particular warning around this part of the code than jump through
> hoops because the compiler is too stupid to understand the code it
> warns about.

Another possible option: you may be able to work around this by
declaring buffer_size to be volatile.

-- 
Michael Welsh Duggan
(md5i@md5i.com)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-23 17:37           ` Eli Zaretskii
  2022-02-23 21:58             ` Michael Welsh Duggan
@ 2022-02-23 22:55             ` Andreas Schwab
  2022-02-24  6:53               ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Andreas Schwab @ 2022-02-23 22:55 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Michael Welsh Duggan, luangruo, emacs-devel

On Feb 23 2022, Eli Zaretskii wrote:

> If the compiler doesn't understand that the value is being limited to
> a maximum of 5 digits, then it shouldn't attempt to emit such
> "helpful" warnings.

Have you tried using the assume macro?

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
       [not found]         ` <87pmndmrsz.fsf@md5i.com>
@ 2022-02-24  0:50           ` Po Lu
  0 siblings, 0 replies; 15+ messages in thread
From: Po Lu @ 2022-02-24  0:50 UTC (permalink / raw)
  To: Michael Welsh Duggan; +Cc: Eli Zaretskii, emacs-devel

Michael Welsh Duggan <mwd@md5i.com> writes:

> Po Lu, what architecture are you compiling for?

As I said, the machine is i586-pc-solaris2.11.




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-23 21:58             ` Michael Welsh Duggan
@ 2022-02-24  6:47               ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2022-02-24  6:47 UTC (permalink / raw)
  To: Michael Welsh Duggan; +Cc: mwd, luangruo, emacs-devel

> From: Michael Welsh Duggan <mwd@md5i.com>
> Cc: Michael Welsh Duggan <mwd@md5i.com>,  luangruo@yahoo.com,
>   emacs-devel@gnu.org
> Date: Wed, 23 Feb 2022 16:58:59 -0500
> 
> > If the compiler doesn't understand that the value is being limited to
> > a maximum of 5 digits, then it shouldn't attempt to emit such
> > "helpful" warnings.
> 
> Is it being limited?  What is limiting it?  "%5.0f" will not limit it's
> size; it will only limit its minimum size, unless I am misunderstanding
> the printf specs.

That's not the limitation I had in mind, I meant the limitation of the
values printed with those formats:

>      if (buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper,
>  				 foreground & 0xFFFFFF, width, height,
> -				 viewbox_width, viewbox_height,
> +				 /* Sanitize the viewBox dimensions.  */
> +				 min (max (viewbox_width, 1.), 10000.),
> +				 min (max (viewbox_height, 1.), 10000.),
>  				 background & 0xFFFFFF,
>  				 SSDATA (encoded_contents)))

Here, it should be clear to the compiler that:

  . the #%06X formats cannot produce more than 6 characters each
  . the %d formats cannot produce more than 12 characters each
  . the %5.0f formats cannot produce more than 5 characters each

> >> The principled way to solve this would be to call the snprintf twice,
> >> the first time with a zero-sized buffer, and then to use the return
> >> value to allocate the actual buffer.  This is a pessimisation, but I
> >> don't know if it's a bad one (it depends on how frequently this code
> >> would be called.
> >
> > This is madness.  I'd rather we used a pragma to disable that
> > particular warning around this part of the code than jump through
> > hoops because the compiler is too stupid to understand the code it
> > warns about.
> 
> Another possible option: you may be able to work around this by
> declaring buffer_size to be volatile.

That'd slow down the code in production, which is not a good idea.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Warning in svg_load_image
  2022-02-23 22:55             ` Andreas Schwab
@ 2022-02-24  6:53               ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2022-02-24  6:53 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: mwd, luangruo, emacs-devel

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Michael Welsh Duggan <mwd@md5i.com>,  luangruo@yahoo.com,
>   emacs-devel@gnu.org
> Date: Wed, 23 Feb 2022 23:55:51 +0100
> 
> On Feb 23 2022, Eli Zaretskii wrote:
> 
> > If the compiler doesn't understand that the value is being limited to
> > a maximum of 5 digits, then it shouldn't attempt to emit such
> > "helpful" warnings.
> 
> Have you tried using the assume macro?

No, I haven't.  I thought about eassert, but rejected the idea,
because aborting a session due to this insignificant issue sounds too
much.  eassume has the same basic problem, so I'm not sure it is a
good idea, either.

Thanks.



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-02-24  6:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <87ilt8hcz4.fsf.ref@yahoo.com>
2022-02-21  7:53 ` Warning in svg_load_image Po Lu
2022-02-21 13:26   ` Eli Zaretskii
2022-02-21 13:37     ` Po Lu
2022-02-21 13:51       ` Eli Zaretskii
2022-02-22  3:53         ` Po Lu
2022-02-22 12:32           ` Eli Zaretskii
2022-02-22 12:45             ` Andreas Schwab
2022-02-22 13:02             ` Po Lu
2022-02-23 17:11         ` Michael Welsh Duggan
2022-02-23 17:37           ` Eli Zaretskii
2022-02-23 21:58             ` Michael Welsh Duggan
2022-02-24  6:47               ` Eli Zaretskii
2022-02-23 22:55             ` Andreas Schwab
2022-02-24  6:53               ` Eli Zaretskii
     [not found]         ` <87pmndmrsz.fsf@md5i.com>
2022-02-24  0:50           ` Po Lu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).