Skip to content

posixfile.cpp: remove redundant realpath() call #13845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pinotree
Copy link
Contributor

Commit 1bd116c makes use of the allocating mode of realpath() by duplicating the realpath() invocation already done (i.e. with the same input path). In practice, if the realpath() invocation with nullptr fails, the realpath() invocation with a destination buffer is very likely to fail in the same way.

Hence drop the realpath() invocation with a destination buffer, and move the realpath() invocation with nullptr in the existing if/else if chain to maintain the existing behaviour.

Since path_buffer now is needed only for the fallback getcwd() invocation, move it in the inner scope where it is used. Since the usage of getcwd() already has a growing buffer logic, make the initial buffer size shorter, which should be enough to get current working directory already at the first attempt in most of the cases.

Commit 1bd116c makes use of the
allocating mode of realpath() by duplicating the realpath() invocation
already done (i.e. with the same input path). In practice, if the
realpath() invocation with nullptr fails, the realpath() invocation with
a destination buffer is very likely to fail in the same way.

Hence drop the realpath() invocation with a destination buffer, and move
the realpath() invocation with nullptr in the existing if/else if chain
to maintain the existing behaviour.

Since "path_buffer" now is needed only for the fallback getcwd()
invocation, move it in the inner scope where it is used. Since the
usage of getcwd() already has a growing buffer logic, make the initial
buffer size shorter, which should be enough to get current working
directory already at the first attempt in most of the cases.
{
dst = &path_buffer[0];
return std::error_condition();
}
else if (path[0] == PATHSEPCH)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this still have the else, which belongs to a now-removed if ()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a chain like

if (cond1) {
   // code for cond1
   return ...
} else if (cond2) {
   // code for cond2
   return ...
} else {
   // code for else
   return ...
}

which is equivalent to

if (cond1) {
   // code for cond1
   return ...
}
if (cond2) {
   // code for cond2
   return ...
}
// code for else
return ...

I replaced the now-removed if called realpath() with a buffer with the if that calls realpath(..., nullptr), as it does the same thing, keeping the if chain as it was for consistency.

@cuavas
Copy link
Member

cuavas commented Jun 20, 2025

The real purpose of this PR seems to be to get rid of PATH_MAX, which as far as I can tell is just to build on GNU Hurd. I don’t think this is beneficial. After 35 years of development, GNU Hurd still has no meaningful presence as a desktop OS (or in any other OS segment for that matter). I don’t think we should be pandering to an OS that no-one uses. I mean, we’ve dropped support for Windows XP and older versions of macOS that have orders of magnitude more users than GNU Hurd.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants