From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Andy Moreton Newsgroups: gmane.emacs.devel Subject: Re: Replace XChar2b with unsigned in all font backends Date: Mon, 20 May 2019 19:29:34 +0100 Message-ID: References: <87v9y5uh63.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="211293"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (windows-nt) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon May 20 20:29:53 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hSn2n-000srh-0D for ged-emacs-devel@m.gmane.org; Mon, 20 May 2019 20:29:53 +0200 Original-Received: from localhost ([127.0.0.1]:40179 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hSn2l-0003CW-UF for ged-emacs-devel@m.gmane.org; Mon, 20 May 2019 14:29:51 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:48355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hSn2e-0003C4-UQ for emacs-devel@gnu.org; Mon, 20 May 2019 14:29:46 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hSn2d-0003tR-9a for emacs-devel@gnu.org; Mon, 20 May 2019 14:29:44 -0400 Original-Received: from [195.159.176.226] (port=56574 helo=blaine.gmane.org) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hSn2d-0003s9-2S for emacs-devel@gnu.org; Mon, 20 May 2019 14:29:43 -0400 Original-Received: from list by blaine.gmane.org with local (Exim 4.89) (envelope-from ) id 1hSn2Z-000shw-RQ for emacs-devel@gnu.org; Mon, 20 May 2019 20:29:39 +0200 X-Injected-Via-Gmane: http://gmane.org/ Cancel-Lock: sha1:EViVhGOr3Q/VpiXo9WC/qww9hV4= X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 195.159.176.226 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:236814 Archived-At: On Mon 20 May 2019, Alex Gramiak wrote: > Andy Moreton writes: > >> Confirmed. This patch replaces use of XChar2b (a 16bit type) with >> unsigned (usually 32bit) which seems wrong. >> >> AndyM > > I originally used unsigned short for this patch, but Eli[1] nudged me > towards unsigned. Unsigned fits better with other parts of the font > system, e.g., *encode_char returns unsigned, and *text_extents takes a > pointer to unsigned. > > [1] https://lists.gnu.org/archive/html/emacs-devel/2019-05/msg00457.html Yes, but Eli was concerned with efficiency, but correctness comes first. If you are calling APIs that expect a pointer to an array of 16bit values, then an array of 32bit values will not suffice. Many of the comments around this code talk of 2-byte values, so changing the code to use 4-byte values is surprising to the reader. The changes also removed the explicit packing/unpacking of 16bit values, which may give rise to endianness issues on some systems. Please revert this patch to fix the build on master, and then revisit these changes after that. AndyM