Subject: bug#8401: removing duplication and improving the readlink code

From: Paul Eggert <[email-masked]>
Date: Fri, 01 Apr 2011 18:57:24 -0700

On 04/01/2011 01:57 PM, Eli Zaretskii wrote:

> this just underlines the difficulty of reading the convoluted
> arrangement that this patch introduces.

Actually, the patch reduces the complexity of Emacs proper; it
removes 58 lines and adds 39 lines.  The patch's core is simple: a
function emacs_readlink, whose body is two lines long, replaces
duplicated code elsewhere in Emacs.

There's nothing that hard about this.  There is some indirection (which
you're calling a "convoluted arrangement"), but that is normal when
replacing inline code with a call to a parameterizable library.  It's
not rocket science.

> this just underlines the difficulty of reading the convoluted
> arrangement that this patch introduces

I would think that only a quick and careless read of the code would
lead one to believe that it calls malloc from Emacs without blocking
interrupts.  However, I wrote the code and perhaps am not the best to
judge its clarity.  A specific suggestion to make the code clearer
(without introducing bugs or reducing flexibility) would be welcome.

> Sorry, that doesn't answer the question.

OK, then the answer is yes, I'm going to seriously consider the case
of Emacs going bad when int or size_t or ssize_t overflows.  This can
occur when the underlying file system is corrupted and reports an incorrect
file size.  I've had this happen personally.  In such cases Emacs
should not crash.

> There should be a good reason for introducing this

There are at least four good reasons.  The change simplifies Emacs's
source code.  It makes Emacs smaller and faster; for example, it
typically reduces the number of system calls (on my RHEL 5.6 host a
patched Emacs typically uses four fewer system calls to lock a file).
The change improves the reliability of Emacs slightly, in unusual
overflow cases.  And the code is written and works.