Hi Jim,
Ben Pfaff noticed that there two versions of xreadlink in gnulib and gettext
http://cvs.savannah.gnu.org/viewcvs/*checkout*/gnulib/lib/xreadlink.c?content-type=text%2Fplain&rev=1.23&root=gnulib
http://cvs.savannah.gnu.org/viewcvs/*checkout*/gettext/gnulib-local/lib/xreadlink.c?content-type=text%2Fplain&rev=1.3&root=gettext
are not compatible.
The one in gettext is derived from your code from coreutils. The difference
is that the one in gnulib has a 'size' argument, the one in gettext
doesn't.
The one in gnulib assumes that stat() has been called before, and minimizes
the readlink() and malloc() calls accordingly: 1 readlink(), 1 malloc() in
the normal case.
The one in gettext assumes no previous operation. It minimizes operations
for this situation: 1 readlink(), 1 malloc(), 1 memcpy() in the normal case.
If the gnulib xreadlink is called when no stat has been done before, like
Ben proposes:
@@ -174,7 +172,7 @@ find_executable (const char *argv0)
{
char *link;
- link = xreadlink ("/proc/self/exe");
+ link = xreadlink ("/proc/self/exe", 4096);
if (link != NULL && link[0] != '[')
return link;
if (executable_fd < 0)
then a buffer is allocated that is way too large: 4 KB even if the link's
contents is just 10 bytes.
Or if one does
link = xreadlink ("/proc/self/exe", 1);
then the result will be several readlink() and several malloc() calls, which
is bad also.
If, on the other hand, the gettext xreadlink is called when stat was called
before, the result (1 readlink(), 1 malloc(), 1 memcpy() in the normal case)
is not as bad, but not optimal either: the memcpy() is unnecessary.
So, in summary, both uses are real use cases, and each has its optimal
implementation.
What can we do in gnulib?
(a) Have 2 xreadlink like functions. Since the one without prior stat is more
natural, I would propose to rename the coreutils one, with the 'size'
argument, to 'xreadlink_after_stat'.
Or
(b) Merge the two functions into one. For example, when the function is called
with size = (size_t)(-1), let it mean "don't know" - and in this case,
let the function use a stack allocated buffer and memcpy().
What do you think?
Bruno
