Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(83208)

#11406: There is no os.listdir() equivalent returning generator instead of list

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years ago by socketpair
Modified:
6 years ago
Reviewers:
thomas, greg, cf.natali
CC:
loewis, Thomas Wouters, rhettinger, terry.reedy, gregory.p.smith, Nick Coghlan, AntoinePitrou, haypo, giampaolo.rodola, christian.heimes, tim.golden, eric.araujo, Trundle, benhoyt, torsten, nvetoshkin, Charles-François Natali, abacabadabacaba_gmail.com, stoneleaf, nailor, socketpair, Josh.R
Visibility:
Public.

Patch Set 1 #

Total comments: 18

Patch Set 2 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/os.rst View 1 2 chunks +39 lines, -1 line 0 comments Download
Lib/importlib/_bootstrap.py View 1 1 chunk +3 lines, -3 lines 0 comments Download
Lib/os.py View 1 2 chunks +25 lines, -0 lines 0 comments Download
Lib/test/test_os.py View 1 9 chunks +47 lines, -7 lines 0 comments Download
Lib/test/test_posix.py View 1 1 chunk +16 lines, -16 lines 0 comments Download
Modules/posixmodule.c View 1 4 chunks +478 lines, -282 lines 4 comments Download
Python/importlib.h View 1 1 chunk +712 lines, -712 lines 0 comments Download

Messages

Total messages: 3
Thomas Wouters
The windows paths are a little broken; see comments. With those things fixed it builds ...
6 years ago #1
gregory.p.smith
http://bugs.python.org/review/11406/diff/7700/Modules/posixmodule.c File Modules/posixmodule.c (right): http://bugs.python.org/review/11406/diff/7700/Modules/posixmodule.c#newcode3264 Modules/posixmodule.c:3264: _listdir_windows_no_opendir(path_t *path, PyObject *list) On 2013/03/31 21:41:45, twouters wrote: ...
6 years ago #2
Charles-François Natali
6 years ago #3
http://bugs.python.org/review/11406/diff/7735/Modules/posixmodule.c
File Modules/posixmodule.c (right):

http://bugs.python.org/review/11406/diff/7735/Modules/posixmodule.c#newcode3608
Modules/posixmodule.c:3608: /* TODO Support returning a namedtuple with dirent
info? issueXXXXX */
I'm not convinced it's a good idea, since only d_name is guaranteed to be
available, so it can't be used portably.

Someone needing more information about the entries could just call stat on them
(or use an hypothetical higher-level walkdir() function). I know it incurs more
syscalls...

http://bugs.python.org/review/11406/diff/7735/Modules/posixmodule.c#newcode3611
Modules/posixmodule.c:3611: unsiged long long d_ino = ep->d_ino;
typo: "unsiged" -> "unsigned"
Also, long long is not ANSI, and long should be enough.

http://bugs.python.org/review/11406/diff/7735/Modules/posixmodule.c#newcode3655
Modules/posixmodule.c:3655: posix_scandir_cleanup(scandir_object *scan)
I think it may not be thread-safe:
since scandir_cleanup is called upon iterator exhaustion, if two threads are
iterating over the same iterator, scan->dirp could be set to NULL and
deallocated while another thread in in the middle of scandir_iter_next.

Using an interator from multiple threads is somewhat brain-dead though...

Note that it will be called implicitely upon deallocation (but I can understand
you want to avoid hanging to the FD for too long).

http://bugs.python.org/review/11406/diff/7735/Modules/posixmodule.c#newcode3661
Modules/posixmodule.c:3661: Py_BLOCK_THREADS
Unless I'm mistaken, the above two lines:
- release the GIL
- acquire the GIL

What's the catch?
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+