Re: [Emacs-diffs] /srv/bzr/emacs/trunk r105581: Integer and memory overflow issues (Bug#9196).

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> +/* Size of header used by bidi_shelve_cache.  */
> +enum
> +  {
> +    bidi_shelve_header_size =
> +      (sizeof (bidi_cache_idx) + sizeof (bidi_cache_start_stack)
> +       + sizeof (bidi_cache_sp) + sizeof (bidi_cache_start)
> +       + sizeof (bidi_cache_last_idx))
> +  };

Why an enum?  Sounds really ugly!

> -	      if (outp - outbuf + MAX_MULTIBYTE_LENGTH * ccl.produced
> -		  > outbufsize)
> +	      /* FIXME: Surely this should be buf_magnification instead.
> +		 MAX_MULTIBYTE_LENGTH overestimates the storage needed.  */
> +	      int magnification = MAX_MULTIBYTE_LENGTH;
> +
> +	      ptrdiff_t offset = outp - outbuf;
> +	      ptrdiff_t shortfall;
> +	      if (INT_MULTIPLY_OVERFLOW (ccl.produced, magnification))
> +		memory_full (SIZE_MAX);
> +	      shortfall = ccl.produced * magnification - (outbufsize - offset);
> +	      if (0 < shortfall)

I don't want to sprinkle memory_full calls all over the code like your
patch does.

Especially since many of the overflow risks it fixes can only happen in
the case where we use 64bit Lisp_Objects on a 32bit system, which is not
currently the default and may end up never being the default (tho it may
also end up being the default, of course).

I.e. right now, those overflows don't worry me as much as the code
cleanliness which seems to suffer from your patch.


        Stefan