XCHAR2B_BYTE[12] definitions in NS

XCHAR2B_BYTE[12] definitions in NS

[Daiki Ueno]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

Hi,

While I'm doing some experiment based on the NS port, I sometimes get
unexpected glyph index, because of missing parentheses in
XCHAR2B_BYTE[12] definitions (patch attached).  I believe those macros
are not actually used by the NS port, but maybe good to fix it to avoid
any confusion.  OK to commit, or perhaps they should be completely
removed?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nsgui.h.patch --]
[-- Type: text/x-patch, Size: 862 bytes --]

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2014-07-14 19:23:18 +0000
+++ src/ChangeLog	2014-07-15 08:48:30 +0000
@@ -1,3 +1,9 @@
+2014-07-15  Daiki Ueno  <ueno@gnu.org>
+
+	* nsgui.h (XCHAR2B_BYTE1): Add missing parentheses around
+	pointer argument, before dereferencing.
+	(XCHAR2B_BYTE2): Likewise.
+
 2014-07-14  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Use binary-io module, O_BINARY, and "b" flag (Bug#18006).

=== modified file 'src/nsgui.h'
--- src/nsgui.h	2014-06-02 18:01:21 +0000
+++ src/nsgui.h	2014-07-15 06:40:08 +0000
@@ -68,10 +68,10 @@
   (*(chp) = ((XChar2b)((((b1) & 0x00ff) << 8) | ((b2) & 0x00ff))))
 
 #define XCHAR2B_BYTE1(chp) \
- (((*chp) & 0xff00) >> 8)
+  ((*(chp) & 0xff00) >> 8)
 
 #define XCHAR2B_BYTE2(chp) \
- ((*chp) & 0x00ff)
+  (*(chp) & 0x00ff)
 
 
 /* XXX: xfaces requires these structures, but the question is are we


[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


Regards,
--
Daiki Ueno

Re: XCHAR2B_BYTE[12] definitions in NS

[Jan Djärv]

Hello.

15 jul 2014 kl. 11:02 skrev Daiki Ueno <ueno@gnu.org>:

> Hi,
> 
> While I'm doing some experiment based on the NS port, I sometimes get
> unexpected glyph index, because of missing parentheses in
> XCHAR2B_BYTE[12] definitions (patch attached).  I believe those macros
> are not actually used by the NS port, but maybe good to fix it to avoid
> any confusion.  OK to commit, or perhaps they should be completely
> removed?
> 

They are used in xdisp.c which is generic code.
W32 has similar macros.  Maybe move to a common h-file and remove w32 and ns-specific macros?

	Jan D.

> === modified file 'src/ChangeLog'
> --- src/ChangeLog	2014-07-14 19:23:18 +0000
> +++ src/ChangeLog	2014-07-15 08:48:30 +0000
> @@ -1,3 +1,9 @@
> +2014-07-15  Daiki Ueno  <ueno@gnu.org>
> +
> +	* nsgui.h (XCHAR2B_BYTE1): Add missing parentheses around
> +	pointer argument, before dereferencing.
> +	(XCHAR2B_BYTE2): Likewise.
> +
> 2014-07-14  Paul Eggert  <eggert@cs.ucla.edu>
> 
> 	Use binary-io module, O_BINARY, and "b" flag (Bug#18006).
> 
> === modified file 'src/nsgui.h'
> --- src/nsgui.h	2014-06-02 18:01:21 +0000
> +++ src/nsgui.h	2014-07-15 06:40:08 +0000
> @@ -68,10 +68,10 @@
>   (*(chp) = ((XChar2b)((((b1) & 0x00ff) << 8) | ((b2) & 0x00ff))))
> 
> #define XCHAR2B_BYTE1(chp) \
> - (((*chp) & 0xff00) >> 8)
> +  ((*(chp) & 0xff00) >> 8)
> 
> #define XCHAR2B_BYTE2(chp) \
> - ((*chp) & 0x00ff)
> +  (*(chp) & 0x00ff)
> 
> 
> /* XXX: xfaces requires these structures, but the question is are we
> 
> 
> Regards,
> --
> Daiki Ueno

Re: XCHAR2B_BYTE[12] definitions in NS

[Daiki Ueno]

Jan Djärv <jan.h.d@swipnet.se> writes:

> They are used in xdisp.c which is generic code.
> W32 has similar macros.  Maybe move to a common h-file and remove w32
> and ns-specific macros?

Makes sense, thanks for the suggestion.  I've pushed a patch in that way
(r117538).

Regards,
--
Daiki Ueno

Re: XCHAR2B_BYTE[12] definitions in NS

[Daiki Ueno]

Daiki Ueno <ueno@gnu.org> writes:

> Jan Djärv <jan.h.d@swipnet.se> writes:
>
>> They are used in xdisp.c which is generic code.
>> W32 has similar macros.  Maybe move to a common h-file and remove w32
>> and ns-specific macros?
>
> Makes sense, thanks for the suggestion.  I've pushed a patch in that way
> (r117538).

I missed the fact that s->char2b is directly passed to the
platform-dependent drawing functions: ExtTextOutW (W32) and
CGContextShowGlyphsWithAdvances (NS), and the corresponding argument
types are also platform-dependent: wchar_t * (W32) and unichar * (NS).

So, IMO merging the macros into a single place will cause more
confusion.  I reverted the change and just installed the parentheses fix
I initially posted.  Sorry for all the fuss.

Regards,
--
Daiki Ueno