On Jul 4, 2019, at 04:44, Pádraig Brady <address@hidden> wrote:
>
> diff --git a/lib/areadlink-with-size.c b/lib/areadlink-with-size.c
> index eacad3f..2fbe51c 100644
> --- a/lib/areadlink-with-size.c
> +++ b/lib/areadlink-with-size.c
> @@ -36,14 +36,15 @@
> check, so it's OK to guess too small on hosts where there is no
> arbitrary limit to symbolic link length. */
> #ifndef SYMLINK_MAX
> -# define SYMLINK_MAX 1024
> +# define SYMLINK_MAX 1023
> #endif
>
> #define MAXSIZE (SIZE_MAX < SSIZE_MAX ? SIZE_MAX : SSIZE_MAX)
>
> /* Call readlink to get the symbolic link value of FILE.
> SIZE is a hint as to how long the link is expected to be;
> - typically it is taken from st_size. It need not be correct.
> + typically it is taken from st_size. It need not be correct,
> + and a value of 0 (or more than 8Ki) will select an appropriate lower bound.
> Return a pointer to that NUL-terminated string in malloc'd storage.
> If readlink fails, malloc fails, or if the link value is longer
> than SSIZE_MAX, return NULL (caller may use errno to diagnose). */
> @@ -61,7 +62,7 @@ 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 && size < initial_limit ? size + 1 : initial_limit;
>
> while (1)
I'd thought of something similar with the 1KB limit, but then I was worried if
"ls" or something is saving all of the symlink targets in memory that using a
too-large buffer size would cause excess memory to be allocated for a long time.
The above logic (AFAICS) is intended to limit the *maximum* allocation size if
"size" is a bogus value, but I don't think initial_limit is a good lower bound.
I picked the lower bound of the malloc() (32 bytes = 4 * sizeof(void *)) as
documented in
https://sourceware.org/glibc/wiki/MallocInternals#What_is_a_Chunk.3F
My version of the patch (merged with your comments) using this size is attached.
My stats show over 99% of symlinks are under 128 bytes, which isn't _too_ much
to allocate for every file. If we want this code to very efficient (minimize
readlink() calls) at the cost of some RAM, it would be enough to use 127 bytes
as the starting point.
Cheers, Andreas
0001-areadlink_with_size-handle-size-0-better.patch