Skip to content

WIP Add resolving of $PROGRAM_NAME from /dev/fd/number #23358

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.

Sign up for GitHub

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: blead
Choose a base branch
from
Open

WIP Add resolving of $PROGRAM_NAME from /dev/fd/number #23358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,7 @@ Michael Somos <[email protected]>
Michael Stevens <[email protected]>
Michael van Elst <[email protected]>
Michael Witten <[email protected]>
Michal Josef Špaček <[email protected]>
Michele Sardo
Michiel Beijen <[email protected]>
Mik Firestone <[email protected]>
Expand Down
13 changes: 13 additions & 0 deletions perl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4227,6 +4227,19 @@ S_open_script(pTHX_ const char *scriptname, bool dosearch, bool *suidscript)
Safefree(PL_origfilename);
PL_origfilename = (char *)scriptname;
}
else {
Copy link
Contributor

Choose a reason for hiding this comment

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

this else { block starting here really should be split off into a separate static, not-inline function, see my other comment below why this is needed

char proc_fd_path[64];
snprintf(proc_fd_path, sizeof(proc_fd_path), "/proc/self/fd/%d", fdscript);
Copy link
Contributor

Choose a reason for hiding this comment

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

"/proc/self/fd/" is 14 +1 chars
"/proc/self/fd/%d" is 16+ 1 chars

"%d" max legal expansion is 21 chars according to google but plz verify it, or find perl.h's or handy.h's official macro for max base10 expansion

"%d" is signed, what does "/proc/self/fd/-13" means on Linux?

The c stack buffer should really be 16+21+1 == 38 or with sizeof(size_t) round up logic, (16+21+1)/8 == 4.75; 5*8 == 40 b/c why not?

The CC will round up C auto var char[38] to the next 4 or 8 alignment mark no matter what you do.

char target_path[MAXPATHLEN];
SSize_t len = readlink(proc_fd_path, target_path, sizeof(target_path) - 1);
if (len != -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

On Linux and FooNix OSes, MAXPATHLEN is constant 1024, 4096, or 2**16 == 65536.

Those 3 buffer lengths are an alligator [S_open_script] taking a bite into a sub/hero/hoagie [C stk].

Since this huge C stk buffer is not used on all control flow paths through S_open_script, but the CC/machine code, needs to stretch the C stack by 1024/4096/65536 bytes each time upon entry, and then use Intel's U32 offsets operands everywhere in the machine code of this function's body, and not Intel's more efficient U8/I8 offset operand encoding choice. It is smarter to put this new else{} branch into its own dedicated static no-line function, so the new else{} branch, and new else{} branch's 1024 or upto 65536 bytes long C stk buffer, don't at runtime, affect the existing control flow paths inside ``S_open_script` that don't know, don't care, and don't use, that 1024-65536 stack buffer.

I have no tech/coding problem with this huge size + very short life span C stack buffer. I don't want to see it become another bloated wasteful Newx()/Safefree()/SAVEFREEPV() statement pair in perl code. I'd would prefer that not all 100% of permutations of all PP calls to PP do/require/use, force that 1024-65K buffer to temporarily exist while sitting unused on the C stack.

On WinPerl because of a P5P bug, CPP macro MAXPATHLEN is 1024 instead of MS's MAX_PATH == 256 or P5P's Win32 cargo cult of writing char bug [ MAX_PATH+1 ];. WinNT Kernel's real path limit is forever frozen at I16_MAX "characters of an undefined encoding", aka, a 32K long array of WCHARs. which becomes 65536 bytes or U16_MAX for the byte length of a 32K long array of WCHARs.

Stat_t statbuf;
target_path[len] = '\0';
if (PerlLIO_stat(target_path, &statbuf) == 0 && S_ISREG(statbuf.st_mode)) {
scriptname = PL_origfilename = find_script(target_path, dosearch, NULL, 1);
}
}
}
}
}

Expand Down
Loading