| Message ID | 20220122144523.2221584-1-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
| Series | tst-realpath-toolong: Fix hurd build | expand |
Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit: > We don't really need a bigger buffer for realpath since it should fail > and return NULL. In the bug too, the buffer itself is not accessed; it > is in fact left untouched. Drop the PATH_MAX use and pass a single char > address. ? realpath assumes that the passed buffer is PATH_MAX-long. When PATH_MAX is not defined, calling it with a buffer is essentially undefined. Better just pass NULL. > Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> > --- > stdlib/tst-realpath-toolong.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c > index 8bed772460..ed84787a32 100644 > --- a/stdlib/tst-realpath-toolong.c > +++ b/stdlib/tst-realpath-toolong.c > @@ -34,8 +34,8 @@ do_test (void) > { > char *base = support_create_and_chdir_toolong_temp_directory (BASENAME); > > - char buf[PATH_MAX + 1]; > - const char *res = realpath (".", buf); > + char buf; > + const char *res = realpath (".", &buf); > > /* canonicalize.c states that if the real path is >= PATH_MAX, then > realpath returns NULL and sets ENAMETOOLONG. */ > -- > 2.34.1 >
On 23/01/2022 06:06, Samuel Thibault wrote: > Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit: >> We don't really need a bigger buffer for realpath since it should fail >> and return NULL. In the bug too, the buffer itself is not accessed; it >> is in fact left untouched. Drop the PATH_MAX use and pass a single char >> address. > > ? realpath assumes that the passed buffer is PATH_MAX-long. When > PATH_MAX is not defined, calling it with a buffer is essentially > undefined. Better just pass NULL. Passing NULL doesn't reproduce the problem because realpath just allocates enough to accommodate the return, even when it exceeds PATH_MAX. It only applies when a non-NULL buffer is supplied. Would you prefer it if I defined PATH_MAX on hurd then, something like: #ifndef PATH_MAX # define PATH_MAX 1024 #endif or do you prefer a more accurate path_max value using pathconf()? The former will be a simpler fix, the latter will be best served by a get_path_max support function, which will be more elaborate but accurate. I'm happy to do either. Thanks, Siddhesh
Siddhesh Poyarekar, le dim. 23 janv. 2022 20:49:48 +0530, a ecrit: > On 23/01/2022 06:06, Samuel Thibault wrote: > > Siddhesh Poyarekar via Libc-alpha, le sam. 22 janv. 2022 20:15:23 +0530, a ecrit: > > > We don't really need a bigger buffer for realpath since it should fail > > > and return NULL. In the bug too, the buffer itself is not accessed; it > > > is in fact left untouched. Drop the PATH_MAX use and pass a single char > > > address. > > > > ? realpath assumes that the passed buffer is PATH_MAX-long. When > > PATH_MAX is not defined, calling it with a buffer is essentially > > undefined. Better just pass NULL. > > Passing NULL doesn't reproduce the problem because realpath just allocates > enough to accommodate the return, even when it exceeds PATH_MAX. Ah, ok, sorry I didn't check the whole story. > Would you prefer it if I defined PATH_MAX on hurd then, something like: > > #ifndef PATH_MAX > # define PATH_MAX 1024 > #endif > > or do you prefer a more accurate path_max value using pathconf()? Better use the accurate from pathconf() ; that being said it will return -1 on the ext2fs filesystem, so we'll have to resort to a hardcoded limit in that case anyway. I'm fine with just setting PATH_MAX by hand here. Samuel
diff --git a/stdlib/tst-realpath-toolong.c b/stdlib/tst-realpath-toolong.c index 8bed772460..ed84787a32 100644 --- a/stdlib/tst-realpath-toolong.c +++ b/stdlib/tst-realpath-toolong.c @@ -34,8 +34,8 @@ do_test (void) { char *base = support_create_and_chdir_toolong_temp_directory (BASENAME); - char buf[PATH_MAX + 1]; - const char *res = realpath (".", buf); + char buf; + const char *res = realpath (".", &buf); /* canonicalize.c states that if the real path is >= PATH_MAX, then realpath returns NULL and sets ENAMETOOLONG. */
We don't really need a bigger buffer for realpath since it should fail and return NULL. In the bug too, the buffer itself is not accessed; it is in fact left untouched. Drop the PATH_MAX use and pass a single char address. Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> --- stdlib/tst-realpath-toolong.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)