[issue32557] allow shutil.disk_usage to take a file path on Windows also
Joe Pamer added the comment: Hi! I decided to try fixing this one as a way to get acquainted with the code base. I went ahead and updated the backing NT C function, but please let me know if you'd prefer I update disk_usage as proposed. Thanks! -- nosy: +jopamer ___ Python tracker <https://bugs.python.org/issue32557> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32557] allow shutil.disk_usage to take a file path on Windows also
Joe Pamer added the comment: Got it - thanks! That sounds good to me, so I'll take a look at how other functions are working around MAX_PATH and update the diff to also avoid the copy when possible. -- ___ Python tracker <https://bugs.python.org/issue32557> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32557] allow shutil.disk_usage to take a file path on Windows also
Joe Pamer added the comment: Just to loop back, I updated the PR to avoid MAX_PATH and only allocate in the "not a directory" case. Thanks for getting back to me so quickly! One question, though, is that it *does* seem like MAX_PATH is still referenced in several places in posixmodule.c. Is the ultimate goal to remove those references as well? -- ___ Python tracker <https://bugs.python.org/issue32557> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue32557] allow shutil.disk_usage to take a file path on Windows also
Joe Pamer added the comment: Awesome - thanks, Steve - this is all super helpful! If you're cool with it I'd like to stick to using _dirnameW for now, and then follow up with another set of PRs for the fixes you've recommended. -- ___ Python tracker <https://bugs.python.org/issue32557> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue34944] Update _dirnameW to accept long path names
New submission from Joe Pamer : The fix for issue 32557 updated os__getdiskusage_impl to use _dirnameW for obtaining the parent directory of a file. This would cause a regression if the path exceeded 260 characters, since _dirnameW currently returns -1 if given a path >= MAX_PATH in length. As suggested in the issue's comments, _dirnameW should be updated to use PathCchRemoveFileSpec when available (on Win8.1 or greater) to avoid throwing an unnecessary error on a long path. Note: If PathCchRemoveFileSpec isn't available, we can call through PathRemoveFileSpecW, which is otherwise deprecated. What's interesting there is that while the docs say it expects a buffer of size MAX_PATH, analysis of the function shows that it doesn't make assumptions about the size of the path other than it's less than 32k characters in length. It calls through PathCchRemoveFileSpec under the hood on Win8 and greater, passing in 0x8000h as the Cch argument. PathCchRemoveFileSpec then scans through the path for '\' via wcschr, stops when it hits the last one and inserts a NULL. (Analysis of PathRemoveFileSpecW on a Win7 VM shows that it does basically the same thing, and is also resilient to paths greater than MAX_PATH in length.) -- components: Windows messages: 327402 nosy: jopamer, paul.moore, steve.dower, tim.golden, zach.ware priority: normal severity: normal status: open title: Update _dirnameW to accept long path names versions: Python 3.8 ___ Python tracker <https://bugs.python.org/issue34944> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue34944] Update _dirnameW to accept long path names
Change by Joe Pamer : -- keywords: +patch pull_requests: +9157 stage: -> patch review ___ Python tracker <https://bugs.python.org/issue34944> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com