Patchwork [2,of,2] win32: do not call GetVolumePathName() with the minimum buffer length

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 2, 2018, 3:40 a.m.
Message ID <c365a5bab6c102b4af74.1514864444@mimosa>
Download mbox | patch
Permalink /patch/26517/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 2, 2018, 3:40 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1514862848 -32400
#      Tue Jan 02 12:14:08 2018 +0900
# Node ID c365a5bab6c102b4af74155ec54eae5d01248f50
# Parent  99cf3126c685bebbac8e7048f9267494d67ead50
win32: do not call GetVolumePathName() with the minimum buffer length

It fails on Windows XP even though the doc says "a safer but slower way to
set the size of the return buffer is to call the GetFullPathName function,
and then make sure that the buffer size is at least the same size as the full
path that GetFullPathName returns."

https://msdn.microsoft.com/en-us/library/windows/desktop/aa364996(v=vs.85).aspx

Well, more "safe" way would be to simply rely on MAX_PATH for common scenarios.
Matt Harbison - Jan. 3, 2018, 1:34 a.m.
On Mon, 01 Jan 2018 22:40:44 -0500, Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1514862848 -32400
> #      Tue Jan 02 12:14:08 2018 +0900
> # Node ID c365a5bab6c102b4af74155ec54eae5d01248f50
> # Parent  99cf3126c685bebbac8e7048f9267494d67ead50
> win32: do not call GetVolumePathName() with the minimum buffer length

LGTM, thanks.
Augie Fackler - Jan. 6, 2018, 8:41 p.m.
On Tue, Jan 02, 2018 at 08:34:35PM -0500, Matt Harbison wrote:
> On Mon, 01 Jan 2018 22:40:44 -0500, Yuya Nishihara <yuya@tcha.org> wrote:
>
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1514862848 -32400
> > #      Tue Jan 02 12:14:08 2018 +0900
> > # Node ID c365a5bab6c102b4af74155ec54eae5d01248f50
> > # Parent  99cf3126c685bebbac8e7048f9267494d67ead50
> > win32: do not call GetVolumePathName() with the minimum buffer length
>
> LGTM, thanks.

Queued per this review. Thanks!

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/win32.py b/mercurial/win32.py
--- a/mercurial/win32.py
+++ b/mercurial/win32.py
@@ -439,7 +439,9 @@  def getvolumename(path):
     # realpath() calls GetFullPathName()
     realpath = os.path.realpath(path)
 
-    size = len(realpath) + 1
+    # allocate at least MAX_PATH long since GetVolumePathName('c:\\', buf, 4)
+    # somehow fails on Windows XP
+    size = max(len(realpath), _MAX_PATH) + 1
     buf = ctypes.create_string_buffer(size)
 
     if not _kernel32.GetVolumePathNameA(realpath, ctypes.byref(buf), size):