emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Emacs-diffs] /srv/bzr/emacs/trunk r105581: Integer and memory overf


From: Stefan Monnier
Subject: Re: [Emacs-diffs] /srv/bzr/emacs/trunk r105581: Integer and memory overflow issues (Bug#9196).
Date: Fri, 26 Aug 2011 23:28:49 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)

> +/* 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]