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.
