unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* LGLYPH_SET_CODE
@ 2008-08-29  8:44 Eli Zaretskii
  2008-08-29 11:17 ` LGLYPH_SET_CODE Kenichi Handa
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2008-08-29  8:44 UTC (permalink / raw)
  To: Kenichi Handa; +Cc: emacs-devel

Can the second argument to this macro be negative?  If so, the current
definition:

    #define LGLYPH_SET_CODE(g, val)					\
      do {								\
	if (val == FONT_INVALID_CODE)					\
	  ASET ((g), LGLYPH_IX_CODE, Qnil);				\
	else if ((EMACS_INT)val > MOST_POSITIVE_FIXNUM)			\
	  ASET ((g), LGLYPH_IX_CODE, Fcons (make_number ((val) >> 16),	\
					    make_number ((val) & 0xFFFF)));\
	else								\
	  ASET ((g), LGLYPH_IX_CODE, make_number (val));		\
      } while (0)

does not handle such values correctly, because their absolute value
could be large enough to overflow an EMACS_INT.  If negative values
are possible, we should use FIXNUM_OVERFLOW_P instead of a comparison
with MOST_POSITIVE_FIXNUM.





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

* Re: LGLYPH_SET_CODE
  2008-08-29  8:44 LGLYPH_SET_CODE Eli Zaretskii
@ 2008-08-29 11:17 ` Kenichi Handa
  2008-08-29 11:26   ` LGLYPH_SET_CODE Jason Rumney
  2008-08-29 23:48   ` LGLYPH_SET_CODE Richard M. Stallman
  0 siblings, 2 replies; 5+ messages in thread
From: Kenichi Handa @ 2008-08-29 11:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

In article <u1w081a7p.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> Can the second argument to this macro be negative?  If so,
> the current definition:

>    #define LGLYPH_SET_CODE(g, val)					\
>      do {								\
>	if (val == FONT_INVALID_CODE)					\
>	  ASET ((g), LGLYPH_IX_CODE, Qnil);				\
>	else if ((EMACS_INT)val > MOST_POSITIVE_FIXNUM)			\
>	  ASET ((g), LGLYPH_IX_CODE, Fcons (make_number ((val) >> 16),	\
>					    make_number ((val) & 0xFFFF)));\
>	else								\
>	  ASET ((g), LGLYPH_IX_CODE, make_number (val));		\
>      } while (0)

> does not handle such values correctly, because their
> absolute value could be large enough to overflow an
> EMACS_INT.  If negative values are possible, we should use
> FIXNUM_OVERFLOW_P instead of a comparison with
> MOST_POSITIVE_FIXNUM.

It is intended that VAL is not negative.  That macro is
called at four places, and except for one place, it is
assured that VAL is never negative.  The exception is in
uniscribe_shape of w32uniscribe.c:

		  LGLYPH_SET_CODE (lglyph, glyphs[j]);

The type of glyphs[j] is "WORD" and I don't know if it's
unsigned or not, I don't know if ScriptShape set a negative
value in it or not.  Anyway, if it's possible that glyphs[j]
becomes negative, that function should be modified not to
call LGLYPH_SET_CODE with negative VAL.

---
Kenichi Handa
handa@ni.aist.go.jp




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

* Re: LGLYPH_SET_CODE
  2008-08-29 11:17 ` LGLYPH_SET_CODE Kenichi Handa
@ 2008-08-29 11:26   ` Jason Rumney
  2008-08-29 23:48   ` LGLYPH_SET_CODE Richard M. Stallman
  1 sibling, 0 replies; 5+ messages in thread
From: Jason Rumney @ 2008-08-29 11:26 UTC (permalink / raw)
  To: Kenichi Handa; +Cc: Eli Zaretskii, emacs-devel

Kenichi Handa wrote:

> It is intended that VAL is not negative.  That macro is
> called at four places, and except for one place, it is
> assured that VAL is never negative.  The exception is in
> uniscribe_shape of w32uniscribe.c:
>
> 		  LGLYPH_SET_CODE (lglyph, glyphs[j]);
>
> The type of glyphs[j] is "WORD" and I don't know if it's
> unsigned or not
>   

WORD is defined as below in the Windows system headers:

typedef unsigned short WORD;

So I think it is safe.





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

* Re: LGLYPH_SET_CODE
  2008-08-29 11:17 ` LGLYPH_SET_CODE Kenichi Handa
  2008-08-29 11:26   ` LGLYPH_SET_CODE Jason Rumney
@ 2008-08-29 23:48   ` Richard M. Stallman
  2008-09-01  5:17     ` LGLYPH_SET_CODE Kenichi Handa
  1 sibling, 1 reply; 5+ messages in thread
From: Richard M. Stallman @ 2008-08-29 23:48 UTC (permalink / raw)
  To: Kenichi Handa; +Cc: eliz, emacs-devel

    It is intended that VAL is not negative.  That macro is
    called at four places, and except for one place, it is
    assured that VAL is never negative.

At least there should be a loud comment next to the definition,
"Make sure VAL is not negative!"




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

* Re: LGLYPH_SET_CODE
  2008-08-29 23:48   ` LGLYPH_SET_CODE Richard M. Stallman
@ 2008-09-01  5:17     ` Kenichi Handa
  0 siblings, 0 replies; 5+ messages in thread
From: Kenichi Handa @ 2008-09-01  5:17 UTC (permalink / raw)
  To: rms; +Cc: eliz, emacs-devel

In article <E1KZDhI-0001fW-F2@fencepost.gnu.org>, "Richard M. Stallman" <rms@gnu.org> writes:

>     It is intended that VAL is not negative.  That macro
> is called at four places, and except for one place, it is
> assured that VAL is never negative.

> At least there should be a loud comment next to the
> definition, "Make sure VAL is not negative!"

Ok, done.

---
Kenichi Handa
handa@ni.aist.go.jp




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

end of thread, other threads:[~2008-09-01  5:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-29  8:44 LGLYPH_SET_CODE Eli Zaretskii
2008-08-29 11:17 ` LGLYPH_SET_CODE Kenichi Handa
2008-08-29 11:26   ` LGLYPH_SET_CODE Jason Rumney
2008-08-29 23:48   ` LGLYPH_SET_CODE Richard M. Stallman
2008-09-01  5:17     ` LGLYPH_SET_CODE Kenichi Handa

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).