On Wed, May 07, 2008 at 09:06:54PM +0200, Denys Vlasenko wrote:
>On Wednesday 07 May 2008 19:11, Carmelo AMOROSO wrote:
>> >> Proposed patch uses user-supplied buffer directly,
>> >> without intermediate on-stack copy.
>> >> This can only make a difference if user supplied
>> >> a buffer which is too small - thus user breaks API.
>> >>
>> >> Failure scenario:
>> >>
>> >> realpath("/link_name", user_buffer)
>> >>
>> >> /link_name -> /very_long_name_which_fits_into_PATH_MAX_and_is_also_a_link
>> >> -> -> /shorter_name
>> >>
>> >> If user will give e.g. 40-char user_buffer, current implementation
>> >> will work, patched one will overflow user_buffer by intermediate name.
>> >>
>> >> This should not be a problem - user must supply PATH_MAX sized buffer,
>> >> and in this case patched version also works correctly.
>> >>
>> >
>> > And the following patch on top of previous one reuses copy_buf[]
>> > for readlink, eliminating link_buf[]. In order to make it work,
>> > "source" pathname is kept at the end of copy_buf, not at the
>> > beginning (so that last NUL byte is the last byte of the copy_buf[]).
>> >
>> > The situation when readlink returns link name which is too long
>> > (so that it overwrites pathname), was resulting in ENAMETOOLONG
>> > error return. This patch does the same - the fact the we now trash
>> > pathname does not matter, as we are not returning it to the user.
>> >
>> > Run tested.
>> >
>> > Can somebody review these patches please?
>> > --
>> > vda
>> >
>>
>> Hi Denys,
>> I'm just now playing with uclibc-trunk (linuxthreads.old implementation)
>> and discovered
>> that your latest change on realpath implementation brakes test-canon
>> test from uclibc test-suite.
>> Indeed, I've run exactly the same on nptl branch (that contains the
>> previous implementation of
>> the realpath function) and it works fine.
>
>Just built and run test here. Result:
>
># ./test-canon
>./test-canon: flunked test 3 (expected `/etc', got `/.local/etc')
>./test-canon: flunked test 4 (expected `/etc', got `/.local/etc')
>./test-canon: flunked test 7 (expected resolved `/etc/doesNotExist', got
>`/.local/etc/doesNotExist')
>./test-canon: flunked test 14 (expected resolved `', got
>`/.1/usr/srcdevel/uclibc/fix/uClibc.t7/test/stdlib/SYMLINK_LOOP')
>./test-canon: flunked test 15 (expected resolved `', got
>`/.1/usr/srcdevel/uclibc/fix/uClibc.t7/test/stdlib/SYMLINK_LOOP')
>./test-canon: flunked test 18 (expected `/etc', got `/.local/etc')
>./test-canon: flunked test 20 (expected `/etc', got `/.local/etc')
>./test-canon: flunked test 22 (expected `/etc', got `/.local/etc')
>./test-canon: flunked test 24 (expected `/etc', got `/.local/etc')
>
>Except 14 and 15, test makes an assumption that /etc is not a symlink.
>On my machine it is not true, so it fails.
>
>Tests 14 and 15 say in source:
>
> /* 10 */
> {"./doesExist/../doesExist", "./doesExist"},
> {"foobar", 0, "./foobar", ENOENT},
> {".", "."},
> {"./foobar", 0, "./foobar", ENOENT},
>#ifdef __UCLIBC__
> /* we differ from glibc here, but POSIX allows it as it says that if we did
> * not successfuly complete, the value of resolved_path is undefined */
> {"SYMLINK_LOOP", 0, "", ELOOP},
> /* 15 */
> {"./SYMLINK_LOOP", 0, "", ELOOP},
>#else
>
>I can make it work so that test passes. However, as this comment says,
>POSIX says nothing about return buffer's contents, so alternative
I didn't (and will not) look what posix mandates in this regard, but if
posix sais that it's undefined then it's just that, undefined.
Obviously, if we can follow bloat-libc in this respect for compatibility
with minor trade-off, then please follow suite (as always) but if we
cannot, then please introduce a(nother) config-knob to follow bloat-libc
in this respect (ISTR that we flags this as glibc-compat in the comment,
fwiw).
>fix would be to not check its contents, only return value and errno.
>
>Do you want me to make readlink return "" or to stop testing
>buffer's contents? Your pick.
cought:
no-bloat-libc-code-here;
_______________________________________________
uClibc mailing list
uClibc@uclibc.org
http://busybox.net/cgi-bin/mailman/listinfo/uclibc
