realpath: Avoid doing unnecessary work

[des]

Summary

The only case in which the userland implementation would return a
different error than the system call is if we are racing against
someone modifying the hierarchy we are traversing, but that is not a
scenario that we are expected to handle gracefully. Therefore,
unless the caller passed in a buffer (which the system call does not
populate), do not call the userland implementation unless the system
call is unavailable.

Sponsored by: Klara, Inc.

[des]

rebase

[markj]

lib/libc/stdlib/realpath.c

234

This res == NULL case only happens when we call realpath1().

[markj]

The only case in which the userland implementation would return a different error than the system call is if we are racing against someone modifying the hierarchy we are traversing

How do you know this? It seems possible that there are some corner cases which __realpathat() simply doesn't handle.

[des]

lib/libc/stdlib/realpath.c

234

True, but I think the code is easier to reason about if I leave it as is.

[des]

In D53026#1211606, @markj wrote:

The only case in which the userland implementation would return a different error than the system call is if we are racing against someone modifying the hierarchy we are traversing

How do you know this? It seems possible that there are some corner cases which __realpathat() simply doesn't handle.

Such as?

[markj]

In D53026#1211680, @des wrote:

In D53026#1211606, @markj wrote:

The only case in which the userland implementation would return a different error than the system call is if we are racing against someone modifying the hierarchy we are traversing

How do you know this? It seems possible that there are some corner cases which __realpathat() simply doesn't handle.

Such as?

I don't know. It's not obvious that the old behaviour wasn't intentional. Looking at the implementation of the syscall, I can't see any reason it would be, so ok.

[des]

@mjg @kib do you agree that there is no reason why the userland implementation of realpath() would succeed after the syscall failed, except in the case of a race?

[kib]

In D53026#1211786, @des wrote:

@mjg @kib do you agree that there is no reason why the userland implementation of realpath() would succeed after the syscall failed, except in the case of a race?

sys__realpath() relies on specific filesystem' VOP_VPTOCNP(), which in principle is allowed to fail in a way that prevents regular (AKA directory dotdot lookup) fallback from happen.
It might be that we could fix it by carefully inspecting errors and standardizing the error returns from the VOP to distinguish a voluntarily refuse to work vs. real error. But right now it was not designed with this in mind.

[mjg]

I think some work can be saved if the errno is ENOENT or EACCESS, but someone(tm) has to make sure errors of the sort are not returned from the stage where the syscall does the reverse lookup.