Refactor lookupPath to avoid recursion. NFC #23017
Conversation
hoodmane
force-pushed
the
lookup-path-nonrecursive
branch
from
b959b5b to
1611617
Compare
src/library_fs.js
Outdated
| // split the absolute path | ||
| var parts = path.split('/').filter((p) => !!p); | ||
| // limit max consecutive symlinks to 40 (SYMLOOP_MAX). | ||
| linkloop: for (var nlinks = 0; nlinks < 40; nlinks ++) { |
There was a problem hiding this comment.
Wow, labeled continue targets!
|
Well it's going to conflict with #22998, I suppose you can pick which one to merge first. |
|
Can you confirm those codesize changes are all due to this change? i.e. can you run the rebase on |
|
There are a lot of similar changes when I run it on main too. |
Maybe try with |
|
Even after |
Hmm.. that is strange/annoying. Maybe just revert those files then, and I can do a rebaseline after this lands. |
sbc100
changed the title
|
OK, I just pushed another rebaseline commit, perhaps this one will match what you are generated on main now? |
|
Nope, even if I check out your rebaseline commit and do a clean install of tot from emsdk I get a bunch of changes in code size by one to ten bytes. |
|
I think this PR fixes the case when you symlink a directory to itself |
|
@sbc100 can this be merged? |
|
Thanks! |
This removes the recursion from `lookupPath` and in my opinion makes the control flow much more explicit and comprehensible.
This removes the recursion from
lookupPathand in my opinion makes the control flow much more explicit and comprehensible.