From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: FACE_FROM_ID vs FACE_OPT_FROM_ID Date: Fri, 24 Jun 2016 11:27:26 +0200 Message-ID: <576CFCFE.8050908@cs.ucla.edu> References: <83d1n73c5z.fsf@gnu.org> <576C7D75.4070401@cs.ucla.edu> <8360sz2cpc.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1466760513 2758 80.91.229.3 (24 Jun 2016 09:28:33 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 24 Jun 2016 09:28:33 +0000 (UTC) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Jun 24 11:28:25 2016 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 1bGNPT-000267-K8 for ged-emacs-devel@m.gmane.org; Fri, 24 Jun 2016 11:28:23 +0200 Original-Received: from localhost ([::1]:42148 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGNPS-0008MT-QP for ged-emacs-devel@m.gmane.org; Fri, 24 Jun 2016 05:28:22 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:54068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGNOq-0008IF-EX for emacs-devel@gnu.org; Fri, 24 Jun 2016 05:27:45 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bGNOo-0005W2-C8 for emacs-devel@gnu.org; Fri, 24 Jun 2016 05:27:43 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:60394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bGNOh-0005V9-Ah; Fri, 24 Jun 2016 05:27:35 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id EC0611614DF; Fri, 24 Jun 2016 02:27:31 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id v1WysASMhHqf; Fri, 24 Jun 2016 02:27:31 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 2CAAE1614DE; Fri, 24 Jun 2016 02:27:31 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id E2YXfWnZ2kTm; Fri, 24 Jun 2016 02:27:31 -0700 (PDT) Original-Received: from [192.168.1.2] (host82-219-dynamic.32-79-r.retail.telecomitalia.it [79.32.219.82]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 645151614D6; Fri, 24 Jun 2016 02:27:30 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 In-Reply-To: <8360sz2cpc.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 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:204721 Archived-At: On 06/24/2016 10:49 AM, Eli Zaretskii wrote: > Yes, and on purpose. If the face pointer cannot be determined from its > ID, we have no other alternative but crash (or abort) anyway. Of course, and FACE_FROM_ID is intended to be used in situations like this. However, in some cases the caller can handle the situation where a face pointer cannot be determined from its ID, and FACE_OPT_FROM_ID is intended to be used in those cases. It is helpful to the reader (at least, to this reader) to easily see which case is which. > When the face ID is not a valid value, as tested against > FRAME_FACE_CACHE (F)->used, ... FACE_FROM_ID(F, ID), ... when compiled > without --enable-checking, will index the FRAME_FACE_CACHE > (F)->face_by_id[] array with an invalid index, and either segfault or > produce some random garbage that will cause trouble elsewhere. So we > are not solving a real problem here ?! Yes we are. The eassert is helpful, since it reliably crashes Emacs when ID is out-of-range, something that is useful when debugging. On all platforms I know of there is no need to add an eassert that the resulting pointer is non-null, since the caller is about to dereference it anyway and that will reliably cause a crash. However, it is helpful to eassert that ID is in range, since most modern platforms lack reliable subscript checking. With FACE_FROM_ID, the ID should never be out-of-range if Emacs is written correctly, so it's OK to omit the runtime check from production code, for performance. This is not true for FACE_OPT_FROM_ID; it is supposed to return NULL when ID is out of range, and callers are supposed to avoid dereferencing the resulting pointer if it is NULL. > just shutting up the (stupid, IMO) warning from a compiler It is not a stupid warning. It is a useful warning. The modern trend in statically-typed languages is to distinguish "possibly-null pointer to X" from "non-null pointer to X". That way, a compile-time check can reliably detect the error of dereferencing null pointers, which is a real problem in many applications (including Emacs). C does not have this notion directly and in general a C compiler cannot detect the error statically. That being said, GCC has reasonable heuristics to check for the error in many cases, and it's useful to enable this checking to catch silly programming errors. (Of course code that you and I write would never have these errors; it's always *other* people's code. :-) > eassert (face); if (!face) { ...} eassert (X) means that X must be nonzero, so there should never be a need for a runtime check !X after a call to eassert (X).