Skip to content

Commit 28383af

Browse files
committed
shared-module/os: Fix os.mkdir('a/b')
This fixes commit a99f942("'/' and '\' are also acceptable ends of the path now") which broke mkdir. The problem is where the directory name is a single letter like this: >>> os.mkdir('a') >>> os.mkdir('a/b') Traceback (most recent call last): File "<stdin>", line 1, in <module> OSError: [Errno 17] File exists >>> os.mkdir('a/bb') >>> I wasn't smart enough to fix this in the oofatfs library, so I did it in the os shared module by creating a path lookup function for the os methods that only deals with directories. I reverted the library change introduced by the aforementioned commit. This means that os.stat and os.rename can't handle trailing slashes. This is to avoid allowing filenames with trailing slashes to pass through. In order to handle trailing slashes for these it would be necessary to check if it really is a directory before stripping. I didn't do this since the original issue was to make os.chdir tolerate trailing slashes. There's an open MicroPython issue #2929 wrt. trailing slashes and mkdir.
1 parent d08747d commit 28383af

File tree

2 files changed

+19
-5
lines changed

2 files changed

+19
-5
lines changed

lib/oofatfs/ff.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2579,8 +2579,8 @@ FRESULT create_name ( /* FR_OK: successful, FR_INVALID_NAME: could not create
25792579
if (w < 0x80 && chk_chr("\"*:<>\?|\x7F", w)) return FR_INVALID_NAME; /* Reject illegal characters for LFN */
25802580
lfn[di++] = w; /* Store the Unicode character */
25812581
}
2582-
cf = ((w < ' ') || ((w == '/' || w == '\\') && p[si+1] < ' ')) ? NS_LAST : 0; /* Set last segment flag if end of the path */
25832582
*path = &p[si]; /* Return pointer to the next segment */
2583+
cf = (w < ' ') ? NS_LAST : 0; /* Set last segment flag if end of the path */
25842584
#if _FS_RPATH != 0
25852585
if ((di == 1 && lfn[di - 1] == '.') ||
25862586
(di == 2 && lfn[di - 1] == '.' && lfn[di - 2] == '.')) { /* Is this segment a dot name? */

shared-module/os/__init__.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,20 @@ STATIC mp_vfs_mount_t *lookup_path(const char* path, mp_obj_t *path_out) {
5050
return vfs;
5151
}
5252

53+
// Strip off trailing slashes to please underlying libraries
54+
STATIC mp_vfs_mount_t *lookup_dir_path(const char* path, mp_obj_t *path_out) {
55+
const char *p_out;
56+
mp_vfs_mount_t *vfs = mp_vfs_lookup_path(path, &p_out);
57+
if (vfs != MP_VFS_NONE && vfs != MP_VFS_ROOT) {
58+
size_t len = strlen(p_out);
59+
while (len > 1 && p_out[len - 1] == '/') {
60+
len--;
61+
}
62+
*path_out = mp_obj_new_str_of_type(&mp_type_str, (const byte*)p_out, len);
63+
}
64+
return vfs;
65+
}
66+
5367
STATIC mp_obj_t mp_vfs_proxy_call(mp_vfs_mount_t *vfs, qstr meth_name, size_t n_args, const mp_obj_t *args) {
5468
if (vfs == MP_VFS_NONE) {
5569
// mount point not found
@@ -69,7 +83,7 @@ STATIC mp_obj_t mp_vfs_proxy_call(mp_vfs_mount_t *vfs, qstr meth_name, size_t n_
6983

7084
void common_hal_os_chdir(const char* path) {
7185
mp_obj_t path_out;
72-
mp_vfs_mount_t *vfs = lookup_path(path, &path_out);
86+
mp_vfs_mount_t *vfs = lookup_dir_path(path, &path_out);
7387
MP_STATE_VM(vfs_cur) = vfs;
7488
if (vfs == MP_VFS_ROOT) {
7589
// If we change to the root dir and a VFS is mounted at the root then
@@ -93,7 +107,7 @@ mp_obj_t common_hal_os_getcwd(void) {
93107

94108
mp_obj_t common_hal_os_listdir(const char* path) {
95109
mp_obj_t path_out;
96-
mp_vfs_mount_t *vfs = lookup_path(path, &path_out);
110+
mp_vfs_mount_t *vfs = lookup_dir_path(path, &path_out);
97111

98112
mp_vfs_ilistdir_it_t iter;
99113
mp_obj_t iter_obj = MP_OBJ_FROM_PTR(&iter);
@@ -120,7 +134,7 @@ mp_obj_t common_hal_os_listdir(const char* path) {
120134

121135
void common_hal_os_mkdir(const char* path) {
122136
mp_obj_t path_out;
123-
mp_vfs_mount_t *vfs = lookup_path(path, &path_out);
137+
mp_vfs_mount_t *vfs = lookup_dir_path(path, &path_out);
124138
if (vfs == MP_VFS_ROOT || (vfs != MP_VFS_NONE && !strcmp(mp_obj_str_get_str(path_out), "/"))) {
125139
mp_raise_OSError(MP_EEXIST);
126140
}
@@ -146,7 +160,7 @@ void common_hal_os_rename(const char* old_path, const char* new_path) {
146160

147161
void common_hal_os_rmdir(const char* path) {
148162
mp_obj_t path_out;
149-
mp_vfs_mount_t *vfs = lookup_path(path, &path_out);
163+
mp_vfs_mount_t *vfs = lookup_dir_path(path, &path_out);
150164
mp_vfs_proxy_call(vfs, MP_QSTR_rmdir, 1, &path_out);
151165
}
152166

0 commit comments

Comments
 (0)