emacs-devel
[Top][All Lists]
Advanced

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

Re: "Final" version of tty child frames


From: Eli Zaretskii
Subject: Re: "Final" version of tty child frames
Date: Tue, 22 Oct 2024 17:40:53 +0300

> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 22 Oct 2024 16:02:39 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Tue, 22 Oct 2024 15:43:29 +0200
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> > Adding a pointer to a number looks kludgey, and might even make the
> >> > hash weaker.
> >> 
> >> If you have something better, please tell. 
> >
> > Give each frame a number, and use that in the hash.
> 
> Thanks, that would work of course. Don't know if it's worth the effort.

Btw, the current code has a problem here:

          int face_id = glyph->face_id;
          [...]
          if (glyph->frame && glyph->frame != f)
            face_id += (ptrdiff_t) glyph->frame->face_cache;
          [...]
          hash = (((hash << 4) + (hash >> 24)) & 0x0fffffff) + c;
          hash = (((hash << 4) + (hash >> 24)) & 0x0fffffff) + face_id;

face_id is an 'int', but 'ptrdiff_t' is a 64-bit data type in 64-bit
builds.  So the addition of face_cache pointer could overflow, which
AFAIK is UB with signed data types.  Moreover, 'hash' is declared
'unsigned int', and so is the data type returned by line_hash_code,
which only makes the problem worse.

Paul, should we make line_hash_code return 'size_t' instead (and
change the data type of the variables in 'scrolling' accordingly)?  Or
maybe we should simply mask off high bits of the face_cache's pointer,
leaving only the low 32 bits, before adding it to the hash?



reply via email to

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