GNU bug report logs - #50767
28.0.50; Warnings about snprintf in image.c on armv7l

Previous Next

Full log

View this message in rfc822 format

From: Alan Third <alan <at> idiocy.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 50767 <at> debbugs.gnu.org
Subject: bug#50767: 28.0.50; Warnings about snprintf in image.c on armv7l
Date: Thu, 23 Sep 2021 23:49:37 +0100

On Thu, Sep 23, 2021 at 11:38:06PM +0100, Basil L. Contovounesios wrote:
> Alan Third [2021-09-23 22:46 +0100] wrote:
> 
> > On Thu, Sep 23, 2021 at 06:04:12PM +0100, Basil L. Contovounesios via Bug
> > reports for GNU Emacs, the Swiss army knife of text editors wrote:
> >>
> >> > const char *css_spec = "svg{font-family:\"%s\";font-size:%4dpx}";
> >> 
> >> Why specifically '%4d' for face_font_size?
> >
> > I figured it unlikely that anyone would be using a font size of 10000
> > pixels or larger and I wanted to set an upper limit on the string size.
> 
> AFAIK %d does not truncate numbers with more digits than the specified
> width or precision...

Hmm, I guess so.

> >> > int css_len = strlen (css_spec) + strlen (img->face_font_family);
> >> > css = xmalloc (css_len);
> >> > snprintf (css, css_len, css_spec, img->face_font_family, img->face_font_size);
> >> > rsvg_handle_set_stylesheet (rsvg_handle, (guint8 *)css, strlen (css), NULL);
> >> 
> >> Does css_len not need to include the terminating null byte?
> >
> > It does. If you add up the length of the spec string which includes
> > the escape codes, and the length of the font name, then if the font
> > size does produce it's maximum sized string of 4 characters css_len is
> > exactly one byte larger than the string length.
> 
> ...which would mean this only holds in the common case that
> face_font_size has fewer than 5 digits, right?

Looks that way.

I suppose in this particular case we could limit the font size to a
maximum of 9999 or something, but surely there's a better way of
calculating string sizes?

> >> What if xmalloc or snprintf fail?
> >
> > Doesn't xmalloc causes some sort of error to occur? I'm not sure.
> 
> I think it exits only while Emacs is still initialising itself before
> entering the top-level command loop, and otherwise frees ballast memory
> and signals a Lisp error.
> 
> At least the second snprintf in svg_load_image does check xmalloc's
> return value.

None of the other uses of xmalloc in image.c check the return value
either, as far as I can see, and I certainly didn't write them all...
-- 
Alan Third

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.