On 06/07/19 00:53, Paul Eggert wrote:
> Pádraig Brady wrote:
>> It would be nice to have areadlink_with_size treat 0 as auto select some
>> lower bound.
>
> Yes, that sounds good. However, I didn't see why that would entail changing
> SYMLINK_MAX from 1024 to 1023, or why the patch would affect the documented
> API.
>
> How about the attached patch instead? When the guessed size is zero it
> typically
> avoids a realloc by using a small stack buffer.
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.
Given Bruno's interim patch of shrinking to the exact sized buffer,
I've push the attached simpler patch that uses a starting buffer
of size 128 (suggested by Andreas), when SIZE==0 is specified.
cheers,
Pádraig
From 0ccc444f3d2dc3ad1b4d682f7d8403633942ed39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sat, 6 Jul 2019 19:43:11 +0100
Subject: [PATCH] areadlink-with-size: guess a buffer size with 0 size
The size is usually taken from st_size, which can be zero,
resulting in inefficient operation as seen with:
$ strace -e readlink stat -c %N /proc/$$/cwd
readlink("/proc/9036/cwd", "/", 1) = 1
readlink("/proc/9036/cwd", "/h", 2) = 2
readlink("/proc/9036/cwd", "/hom", 4) = 4
readlink("/proc/9036/cwd", "/home/pa", 8) = 8
readlink("/proc/9036/cwd", "/home/padraig", 16) = 13
Instead let zero select an initial memory allocation
of 128 bytes, which most symlinks fit within.
* lib/areadlink-with-size.c (areadlink_with_size):
Start with a 128 byte buffer, for SIZE == 0.
* lib/areadlinkat-with-size.c (areadlinkat_with_size): Likewise.
---
ChangeLog | 11 +++++++++++
lib/areadlink-with-size.c | 3 ++-
lib/areadlinkat-with-size.c | 3 ++-
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 885907d..0b06131 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2019-07-06 Pádraig Brady <p...@draigbrady.com>
+
+ areadlink-with-size: guess a buffer size with 0 size
+ The size is usually taken from st_size, which can be zero,
+ resulting in inefficient operation.
+ Instead let zero select an initial memory allocation
+ of 128 bytes, which most symlinks fit within.
+ * lib/areadlink-with-size.c (areadlink_with_size):
+ Start with a 128 byte buffer, for SIZE == 0.
+ * lib/areadlinkat-with-size.c (areadlinkat_with_size): Likewise.
+
2019-07-06 Konstantin Kharlamov <hi-an...@yandex.ru>
Replace manually crafted hex regexes with [:xdigit:]
diff --git a/lib/areadlink-with-size.c b/lib/areadlink-with-size.c
index 364cc08..b9cd05c 100644
--- a/lib/areadlink-with-size.c
+++ b/lib/areadlink-with-size.c
@@ -61,7 +61,8 @@ areadlink_with_size (char const *file, size_t size)
: INITIAL_LIMIT_BOUND);
/* The initial buffer size for the link value. */
- size_t buf_size = size < initial_limit ? size + 1 : initial_limit;
+ size_t buf_size = (size == 0 ? 128
+ : size < initial_limit ? size + 1 : initial_limit);
while (1)
{
diff --git a/lib/areadlinkat-with-size.c b/lib/areadlinkat-with-size.c
index 5b2bccc..d39096f 100644
--- a/lib/areadlinkat-with-size.c
+++ b/lib/areadlinkat-with-size.c
@@ -66,7 +66,8 @@ areadlinkat_with_size (int fd, char const *file, size_t size)
: INITIAL_LIMIT_BOUND);
/* The initial buffer size for the link value. */
- size_t buf_size = size < initial_limit ? size + 1 : initial_limit;
+ size_t buf_size = (size == 0 ? 128
+ : size < initial_limit ? size + 1 : initial_limit);
while (1)
{
--
2.9.3
