Re: proposed new module breadlinkat

[Paul Eggert]

On 03/29/2011 04:29 AM, Bruno Haible wrote:

> That looks like a lot of complexity that is useful only to Emacs and
> not to other packages.

I should have explained that I had another agenda here: improving the
performance of programs like coreutils, which invoke areadlink on a
non-huge link, do a bit of processing, and then immediately free the
resulting pointer.  For this common case it'd be nice if the
application didn't have to invoke malloc and free at all.  And
breadlinkat would give coreutils that option.

> sounds like an ancient (signal based) approach to multiprocessing.
> Nowadays thread primitives exist and work on all platforms.

Yes, and Emacs is (slowly) going multithreaded.  However, this will
take quite some time (many years, probably), and the current direction
is not clear; certainly it's not clear that even when it's done the
Emacs folks will want to give up on allocating memory in a signal
handler.  Let's keep readlink decoupled from this thorny issue.

> Because all that you effectively need is a patch like this:

It'd have to be more complicated than that, because the patch would
have to declare Emacs's xmalloc and xrealloc functions, which would
mean it'd have to pull in Emacs's lisp.h, which is in some other
directory, and ... well, there are other options, but in the long run
I expect it'll be better to solve the problem directly, particularly
given that it'll also be useful outside Emacs.

>   - Choice of a good name: It's not clear what the prefix 'b' means.
>     The 'a' in 'areadlink' means "allocating". As I understand it here,
>     the purpose is a "readlink with customizable allocation". So
>     'careadlink' would be a better name, no?

'b' meant "buffer" (i.e., perhaps no malloc at all), but "ca" is fine too.

>     it would make sense
>     to group the 4 function pointers in a single 'struct'. In C++ this
>     concept is called "allocator" [3].

Yes, we could create an auxiliary module "allocator", with an
allocator.h, or something like that.  I'll look into it.

>   - Update the module dependency list in lib/relocwrapper.c and the file list
>     in modules/relocatable-prog-wrapper.

Sorry, what modification would be needed there?  breadlinkat.c doesn't
link to malloc/free/etc. so why would relocwrapper need to be changed?
(Sorry, I don't fully understand relocwrapper.)  Are you worried about
link-time optimization, say?

> Fix a link error in the relocwrapper module. I had verified that
> areadlink.c passes only positive sizes to malloc and realloc. The same
> verification also holds for areadlinkat.c. So, adding
>   #undef malloc
>   #undef realloc
> in areadlinkat.c would be only a micro-optimization, not a fix of a link
> error. Feel free to add it, though, if you find it appropriate.

Thanks for explaining.  If the #undef is not needed for areadlinkat.c
then let's leave it alone.