[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


From: Paul Eggert
Date: Fri, 24 Jun 2016 11:27:26 +0200
User-agent: Mozilla/5.0 (X11; Linux i686; rv:38.0) Gecko/20100101 Thunderbird/38.8.0

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]