← index #17012PR #12137
Likely Duplicate · high · value 2.393
QUERY · ISSUE

vfs.VfsPosix('.') gets confused if cwd changes

openby dubiousjimopened 2025-03-26updated 2025-03-27
bug

Port, board and/or hardware

Unix port, installed on Alpine Linux (uses musl libc)

MicroPython version

MicroPython v1.24.1 on 2025-03-14; linux [GCC 14.2.0] version

If I create a VfsPosix object rooted at '.', and query the working directory with the getcwd() method, I get a sane result (""). If I then change the working directory, either by using the chdir() method on that object, or using the os module, and query the working directory again through the VfsPosix object, I get an invalid result (just some trailing characters from the correct result). This invalid result persists (that is, it doesn't update) even if I chdir() a second time.

Reproduction

main0:~/micropython-work/test$ tree
.
└── foo

1 directories, 0 files

$ micropython
>>> import os,vfs; Vfs=vfs.VfsPosix
>>> os.getcwd()
'/home/bob/micropython-work/test'
>>> v=Vfs('.')
>>> v.getcwd()
''
>>> os.chdir('foo')
>>> os.getcwd() # output as expected
'/home/bob/micropython-work/test/foo'
>>> v.getcwd() # weird output
'oo'
>>> os.chdir('..')
>>> os.getcwd() # output as expected
'/home/bob/micropython-work/test'
>>> v.getcwd() # weird output persists
'oo'

Expected behaviour

Expected second v.getcwd() call to return something like "foo" or "./foo" or "/foo". Expected third v.getcwd() call to return something like "" again.

Observed behaviour

Output given in Reproduction.

Additional Information

No, I've provided everything above.

Code of Conduct

Yes, I agree

CANDIDATE · PULL REQUEST

extmod/vfs_posix: Fix relative paths on non-root VFS

mergedby cwaltheropened 2023-07-31updated 2023-10-20
extmod

VfsPosix has a couple of bugs regarding handling of relative paths and chdir()/getcwd() that surface when it is instanced with a non-empty root path (i.e. only a subtree of the outer filesystem is exposed to Python).

They are somewhat entangled, but I tried my best to separate the fixes into individually reviewable commits, each with first a failing test that demonstrates what is going wrong, followed by fixes that makes it pass. (The tricky part of that was to get the fixes in the right order and craft the tests such that they actually pass after the respective fix, rather than continuing to fail due to the remaining bugs.)

  1. A VfsPosix created with a relative root path gets confused when chdir() is called on it and becomes unable to properly resolve absolute paths, because changing directories effectively shifts its root. The simplest fix for that would be to say “don’t do that”, but since the unit tests themselves do it, fix it by making a relative path absolute before storing it.

  2. The unwritten API contract expected of a VFS by mp_vfs_lookup_path() is that paths passed in are relative to the root of the VFS if they start with / and relative to the current directory of the VFS otherwise. This is not correctly implemented in VfsPosix for instances with a non-empty root – all paths are interpreted relative to the root. Fix that. Since VfsPosix tracks its CWD using the “external” CWD of the Unix process, the correct handling for relative paths is to pass them through unmodified.

  3. The unwritten API contract expected of a VFS.getcwd() by mp_vfs_getcwd() is that its return value should be either "" or "/" when the CWD is at the root of the VFS and otherwise start with a slash and not end with a slash. This is not correctly implemented in VfsPosix for instances with a non-empty root – the required leading slash, if any, is cut off because the root length includes a trailing slash. This results in missing slashes in the middle of the return value of os.getcwd() or in uninitialized garbage from beyond a string’s null terminator when the CWD is at the VFS root.

The matter is complicated by the fact that there are some faulty existing tests that only pass by accident due to bug 2. Unmodified, they would fail after the fix for bug 2 (5th commit), however that is insignificant because they actually test an unrealistic situation. Fixing them in the 3rd commit to match the realistic situation makes them pass with bug 2 fixed (and also without, when done in this order, because after the 2nd commit, bug 2 has no effect in that case).

The unrealistic situation is that VfsPosix methods are called with relative paths while the current working directory is somewhere outside of the root of the VFS. In the intended use of VFS objects via os.mount() (as opposed to calling methods directly as the tests do), this never happens, as mp_vfs_lookup_path() directs incoming calls to the VFS that contains the CWD. Fix this by explicitly changing the working directory to the root of the VFS before calling methods on it, as the subsequent relative path accesses expect.

Keyboard

j / / n
next pair
k / / p
previous pair
1 / / h
show query pane
2 / / l
show candidate pane
c
copy suggested comment
r
toggle reasoning
g i
go to index
?
show this help
esc
close overlays

press ? or esc to close

copied