[PATCH 6 of 7 resend] tests: ensure that C and Python path encoding agree

[Matt Mackall]

On Sat, 2013-01-19 at 19:08 +0100, Adrian Buehlmann wrote:
> On 2013-01-17 22:28, Matt Mackall wrote:
> > On Wed, 2012-12-12 at 13:11 -0800, Bryan O'Sullivan wrote:
> >> # HG changeset patch
> >> # User Bryan O'Sullivan <bryano at fb.com>
> >> # Date 1355346577 28800
> >> # Node ID b23eec6a55ee3292b46abdb980adf9d9096d3e1f
> >> # Parent  9fcae4777e797bd8ec8f968b4fde7a148e0963c0
> >> tests: ensure that C and Python path encoding agree
> > 
> > This series is queued for default, except for this one that collides
> > with an earlier patch from Adrian. Thanks!
> 
> Ugh. I was somehow hoping you would have waited with including these after 2.5...

You did have plenty of opportunity to review this last version before I
queued it. It's been in-progress since before 2.4, after all. The
current version has this:

#ifndef _MSC_VER
	/* alloca is surprisingly slow, so avoid when possible */
        char dired[baselen];
        char lowered[baselen];
        char auxed[baselen];
#else
        char *dired = alloca(baselen);
        char *lowered = alloca(baselen);
        char *auxed = alloca(baselen);
#endif

..so some effort was made to make it work for you.

But in any case, this code is suspicious. There are no maximum path
lengths enforced by Mercurial. And what does alloca() do if it can't
fulfill a request? Undefined. Similar for variable arrays.

I've traded this for fixed-sized arrays. This should actually be better
performance-wise and anywhere it would break would also potentially
break just running the test suite anyway. Systems where stack sizes are
constrained such that this matters should be pretty rare.

diff -r d6b3b36f1db2 mercurial/pathencode.c
--- a/mercurial/pathencode.c	Sat Jan 19 02:29:56 2013 +0100
+++ b/mercurial/pathencode.c	Sat Jan 19 17:21:23 2013 -0600
@@ -696,22 +696,22 @@
 	return 0;
 }
 
+#define MAXENCODE 4096 * 3
+
 static PyObject *hashencode(const char *src, Py_ssize_t len)
 {
-	const Py_ssize_t baselen = (len - 5) * 3;
-#ifndef _MSC_VER
-	/* alloca is surprisingly slow, so avoid when possible */
-	char dired[baselen];
-	char lowered[baselen];
-	char auxed[baselen];
-#else
-	char *dired = alloca(baselen);
-	char *lowered = alloca(baselen);
-	char *auxed = alloca(baselen);
-#endif
-	Py_ssize_t dirlen, lowerlen, auxlen;
+	char dired[MAXENCODE];
+	char lowered[MAXENCODE];
+	char auxed[MAXENCODE];
+	Py_ssize_t dirlen, lowerlen, auxlen, baselen;
 	char sha[20];
 
+	baselen = (len - 5) * 3;
+	if (baselen >= MAXENCODE) {
+		PyErr_SetString(PyExc_ValueError, "string too long");
+		return NULL;
+	}
+
 	dirlen = _encodedir(dired, baselen, src, len);
 	if (sha1hash(sha, dired, dirlen - 1) == -1)
 		return NULL;


-- 
Mathematics is the supreme nostalgia of our time.