From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Fabrice Popineau Newsgroups: gmane.emacs.devel Subject: Re: Windows 64 port Date: Tue, 28 Feb 2012 22:00:37 +0100 Message-ID: References: <20120219211800.0000558f@unknown> <834numv7js.fsf@gnu.org> <4F428780.8070902@cs.ucla.edu> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=0015173ff3e2fedae804ba0c85c0 X-Trace: dough.gmane.org 1330478024 5280 80.91.229.3 (29 Feb 2012 01:13:44 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 29 Feb 2012 01:13:44 +0000 (UTC) Cc: Eli Zaretskii , AJMR , emacs-devel@gnu.org To: Paul Eggert Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Feb 29 02:13:43 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1S2Y6x-0004Po-0T for ged-emacs-devel@m.gmane.org; Wed, 29 Feb 2012 02:13:43 +0100 Original-Received: from localhost ([::1]:41932 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2Y6w-0000LZ-Ae for ged-emacs-devel@m.gmane.org; Tue, 28 Feb 2012 20:13:42 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:48722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2UAc-00060G-4z for emacs-devel@gnu.org; Tue, 28 Feb 2012 16:01:15 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S2UAX-0002E9-7W for emacs-devel@gnu.org; Tue, 28 Feb 2012 16:01:13 -0500 Original-Received: from mail-bk0-f41.google.com ([209.85.214.41]:61113) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2UAP-0002CC-2f; Tue, 28 Feb 2012 16:01:01 -0500 Original-Received: by bkty12 with SMTP id y12so6411438bkt.0 for ; Tue, 28 Feb 2012 13:00:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:cc:content-type; bh=oEzU9j7rswDgu0MCkjssZBYnVD70+oAEXbtTZwXKkeA=; b=o7I5ScD2PrKy6MjBU0ihFPbMeGTEQWeXsXjVOQMqjXOQ7QOK6E5ymCzgsOsX4ThZEw lV4u7Ozl8ri5PzgutL4xoMHYJkqcZ9ka2Rc+50r1MSpZQud+3g0dxIQ8L6jc7geDNdHN utTZLr/9yYRR//O6NyYlI8MCU5EWPfhozP10Y= Original-Received: by 10.204.128.211 with SMTP id l19mr8684506bks.59.1330462857288; Tue, 28 Feb 2012 13:00:57 -0800 (PST) Original-Received: by 10.204.60.3 with HTTP; Tue, 28 Feb 2012 13:00:37 -0800 (PST) In-Reply-To: <4F428780.8070902@cs.ucla.edu> X-Google-Sender-Auth: Wwtv5q0SHNMsMPQt25jGOr01ROI X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.214.41 X-Mailman-Approved-At: Tue, 28 Feb 2012 20:13:39 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:148832 Archived-At: --0015173ff3e2fedae804ba0c85c0 Content-Type: text/plain; charset=ISO-8859-1 > > The patch is too intrusive to the mainline code > and many of its changes should be omitted. > I maintain my point of view. Most of my patch is to ensure that 32bits and 64bits values are not mixed up, and that unsigned and signed values aren't either. Maybe some of it can be omitted, that doesn't mean one day or another they won't be missed. > > - int i = 0, aligned = (intptr_t) ABLOCKS_BUSY (abase); > + int i = 0; > + intptr_t aligned = (intptr_t) ABLOCKS_BUSY (abase); > > Here, the value of 'aligned' is either 0 or 1, so there's > no need to change its type. > Then int aligned = (ABLOCKS_BUSY(abase) != NULL); would have been cleaner. The changes to src/m/amdx86-64.h would break GNU/Linux and > need to be backed out and redone. Obviously, they need to be made dependent on the target platform. (Done in the patch to come). > This mishandles Emacs integers outside the signed 32-bit range: > > -#define XFASTINT(a) ((a) + 0) > +#define XFASTINT(a) ((int)((a) + 0)) > Oops. Sorry for this one. It was a leftover. There is something wrong around lib/strftime.c:946 if (negative_number) u_number_value = - u_number_value; u_number_value being unsigned, this is wrong. I haven't look for a proper fix. In src/dispnew.c:6402, height and width should probably be unsigned. The checking by INT_ADD_RANGE_OVERFLOW results in a compiler warning about integral constant overflow because it tries to compute (INTMIN - 2) which obviously is out of range. The value is not used in this case, but the compiler may emit the warning anyway. Best regards, -- Fabrice --0015173ff3e2fedae804ba0c85c0 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
The patch is too = intrusive to the mainline code
and many of its changes should be omitted.

I maintain my point of view. Most of my pa= tch is to ensure that 32bits and 64bits values are not mixed up,=A0
and that unsigned and signed values aren't either. Maybe some of it = can be omitted, that doesn't mean one day
or another they won't be missed.

- =A0 =A0 =A0int i =3D 0, aligned =3D (intptr_t) ABLOCKS_BUSY (abase);
+ =A0 =A0 =A0int i =3D 0;
+ =A0 =A0 =A0intptr_t aligned =3D (intptr_t) ABLOCKS_BUSY (abase);

Here, the value of 'aligned' is either 0 or 1, so there's
no need to change its type.

Then
<= div>
int aligned =3D (ABLOCKS_BUSY(abase) !=3D NULL);

would have been cleaner.

The changes to src/m/amdx86-64.h would break GNU/Linux and
need to be backed out and redone.

Obviously= , they need to be made dependent on the target platform.
(Done in= the patch to come).
=A0
This mishandles Emacs integers outside the signed 32-bit range:

-#define XFASTINT(a) ((a) + 0)
+#define XFASTINT(a) ((int)((a) + 0))

Oops. Sorry for this one. It was a leftover.
There is something wrong around lib/strftime.c:946=A0
=
=A0 =A0 =A0 =A0 =A0 if (negative_number)
=A0 =A0 =A0 =A0 =A0= =A0 u_number_value =3D - u_number_value;

u_number_value being unsigned, this is wrong. I h= aven't look for a proper fix.

In src/dispnew.c= :6402, height and width should probably be unsigned. The checking by
INT_ADD_RANGE_OVERFLOW results in a compiler warning about integral co= nstant overflow
because it tries to compute (INTMIN - 2) which ob= viously is out of range.=A0
The value is not used in this case, b= ut the compiler may emit the warning anyway.

Best regards,

--
Fabrice
--0015173ff3e2fedae804ba0c85c0--