Pádraig Brady wrote:
Your patch has the advantage of allocating the exact right sized buffer in the usual case, but the disadvantage of CPU overhead in string length determination, and some extra code complexity in the separate small buffer handling.
I think the code complexity is worth it, to avoid calling realloc in the typical case when the size is not known. The string length determination can easily be avoided, so I installed the attached which does that.
>From b979980a653ce610fe5f64935adedda425da3ca4 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sat, 6 Jul 2019 19:25:24 -0700 Subject: [PATCH] areadlink-with-size: avoid realloc when size==0 * lib/areadlink-with-size.c (areadlink_with_size): * lib/areadlinkat-with-size.c (areadlinkat_with_size): Reallocate at the end to the actual size, to avoid memory waste, as suggested by Bruno Haible. But when the guessed size is zero - useful when the size is unknown - do the initial small readlink into the stack, to avoid that realloc in the usual case. --- ChangeLog | 10 ++++++++++ lib/areadlink-with-size.c | 31 +++++++++++++++++++++++-------- lib/areadlinkat-with-size.c | 31 +++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0b061317c..f7e031d9b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2019-07-06 Paul Eggert <[email protected]> + + areadlink-with-size: avoid realloc when size==0 + * lib/areadlink-with-size.c (areadlink_with_size): + * lib/areadlinkat-with-size.c (areadlinkat_with_size): + Reallocate at the end to the actual size, to avoid memory waste, + as suggested by Bruno Haible. But when the guessed size is zero - + useful when the size is unknown - do the initial small readlink + into the stack, to avoid that realloc in the usual case. + 2019-07-06 Pádraig Brady <[email protected]> areadlink-with-size: guess a buffer size with 0 size diff --git a/lib/areadlink-with-size.c b/lib/areadlink-with-size.c index b9cd05cba..ae3c782a4 100644 --- a/lib/areadlink-with-size.c +++ b/lib/areadlink-with-size.c @@ -60,19 +60,28 @@ areadlink_with_size (char const *file, size_t size) ? symlink_max + 1 : INITIAL_LIMIT_BOUND); + enum { stackbuf_size = 128 }; + /* The initial buffer size for the link value. */ - size_t buf_size = (size == 0 ? 128 + size_t buf_size = (size == 0 ? stackbuf_size : size < initial_limit ? size + 1 : initial_limit); while (1) { ssize_t r; size_t link_length; - char *buffer = malloc (buf_size); + char stackbuf[stackbuf_size]; + char *buf = stackbuf; + char *buffer = NULL; + + if (! (size == 0 && buf_size == stackbuf_size)) + { + buf = buffer = malloc (buf_size); + if (!buffer) + return NULL; + } - if (buffer == NULL) - return NULL; - r = readlink (file, buffer, buf_size); + r = readlink (file, buf, buf_size); link_length = r; /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1 @@ -87,10 +96,16 @@ areadlink_with_size (char const *file, size_t size) if (link_length < buf_size) { - buffer[link_length] = 0; - /* Shrink BUFFER before returning it. */ - if (link_length + 1 < buf_size) + buf[link_length] = 0; + if (!buffer) + { + buffer = malloc (link_length + 1); + if (buffer) + return memcpy (buffer, buf, link_length + 1); + } + else if (link_length + 1 < buf_size) { + /* Shrink BUFFER before returning it. */ char *shrinked_buffer = realloc (buffer, link_length + 1); if (shrinked_buffer != NULL) buffer = shrinked_buffer; diff --git a/lib/areadlinkat-with-size.c b/lib/areadlinkat-with-size.c index d39096f2b..ed46d5911 100644 --- a/lib/areadlinkat-with-size.c +++ b/lib/areadlinkat-with-size.c @@ -65,19 +65,28 @@ areadlinkat_with_size (int fd, char const *file, size_t size) ? symlink_max + 1 : INITIAL_LIMIT_BOUND); + enum { stackbuf_size = 128 }; + /* The initial buffer size for the link value. */ - size_t buf_size = (size == 0 ? 128 + size_t buf_size = (size == 0 ? stackbuf_size : size < initial_limit ? size + 1 : initial_limit); while (1) { ssize_t r; size_t link_length; - char *buffer = malloc (buf_size); + char stackbuf[stackbuf_size]; + char *buf = stackbuf; + char *buffer = NULL; + + if (! (size == 0 && buf_size == stackbuf_size)) + { + buf = buffer = malloc (buf_size); + if (!buffer) + return NULL; + } - if (buffer == NULL) - return NULL; - r = readlinkat (fd, file, buffer, buf_size); + r = readlinkat (fd, file, buf, buf_size); link_length = r; /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1 @@ -92,10 +101,16 @@ areadlinkat_with_size (int fd, char const *file, size_t size) if (link_length < buf_size) { - buffer[link_length] = 0; - /* Shrink BUFFER before returning it. */ - if (link_length + 1 < buf_size) + buf[link_length] = 0; + if (!buffer) + { + buffer = malloc (link_length + 1); + if (buffer) + return memcpy (buffer, buf, link_length + 1); + } + else if (link_length + 1 < buf_size) { + /* Shrink BUFFER before returning it. */ char *shrinked_buffer = realloc (buffer, link_length + 1); if (shrinked_buffer != NULL) buffer = shrinked_buffer; -- 2.17.1
