From: Paul Eggert <eggert@cs.ucla.edu>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 8401@debbugs.gnu.org
Subject: bug#8401: removing duplication and improving the readlink code
Date: Fri, 01 Apr 2011 13:09:22 -0700 [thread overview]
Message-ID: <4D9630F2.1010806@cs.ucla.edu> (raw)
In-Reply-To: <83hbahyd64.fsf@gnu.org>
On 04/01/2011 12:38 PM, Eli Zaretskii wrote:
> the functions you introduce use malloc/realloc,
> which I think will prevent their callers from being safe for
> asynchronous calls triggered by external events (mouse etc.).
No, because the functions always use Emacs xmalloc / xrealloc when
Emacs invokes them. This is because emacs_readlink tells the
lower-level primitives to use xmalloc and xrealloc.
>> That doesn't suffice; the code should not only use ssize_t for
>> readlink's returned value, but it should also use size_t for the
>> buffer size
>
> Well, let's use that as well, then.
What I sense that you're suggesting, is that we take all the fixes in
the gnulib code, and port them all into the two duplicates of similar
code that's in Emacs. That would be an error-prone process and would
leave us with three copies of the code to maintain. It's better to
have just one copy of the code.
> Are we really going to consider seriously the case of a link name
> overflowing a 64-bit ssize_t data type?
As a general rule, GNU code shouldn't have arbitrary limits, and
should defend against limits in underyling systems. Emacs is not
as good as it should be about this, and I'm trying to make it better.
This is just one example of many, found by static checking, and we
might as well fix it while we're fixing all the others.
> But why do we need to introduce the allocator and careadlinkat
> modules, and all the nested function calls needed for them, just to
> protect a simple code fragment from overflowing?
It's more reliable to put the overflow checks in one place, where they
can be carefully checked, than to duplicate the code in multiple
places where it's easy for programmers to get it wrong.
> We could refactor the duplicated code into a short function, and use
> that.
That's what's being done here. The function does even more than that,
though, as it avoids the need to call malloc and free entirely, in the
usual case. This is a performance win in the typical case.
> please restructure the code so that neither allocator.h nor
> careadlinkat are not used on platforms whose readlink is an
> always-fail stub.
That's not possible to do, unless we add more "#ifdef DOS_NT"s to the
mainstream code. But that would be undesirable. The mainstream code
should be written for the usual case, and platforms that lack the
necessary primitives should supply their own stubs (which may always
fail, but that's OK). That is standard porting technology, and it's
an improvement over sprinking #ifdefs throughout the mainstream code.
next prev parent reply other threads:[~2011-04-01 20:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-01 6:47 bug#8401: removing duplication and improving the readlink code Paul Eggert
2011-04-01 8:33 ` Eli Zaretskii
2011-04-01 19:00 ` Paul Eggert
2011-04-01 19:38 ` Eli Zaretskii
2011-04-01 20:09 ` Paul Eggert [this message]
2011-04-01 20:57 ` Eli Zaretskii
2011-04-02 1:57 ` Paul Eggert
2011-04-03 16:41 ` Stefan Monnier
2011-04-04 4:38 ` Paul Eggert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D9630F2.1010806@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=8401@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.