Am 28.07.2014 um 23:42 schrieb Junio C Hamano:
Jeff King <> writes:

On Mon, Jul 28, 2014 at 08:28:30PM +0200, René Scharfe wrote:

@@ -60,26 +58,22 @@ static const char *real_path_internal(const char *path, int 
                        goto error_out;

-       if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {
-               if (die_on_error)
-                       die("Too long path: %.*s", 60, path);
-               else
-                       goto error_out;
-       }
+       strbuf_init(&sb, 0);
+       strbuf_addstr(&sb, path);

As with the other patch I just mentioned, should this be strbuf_reset,
not strbuf_init? We want to reset the static buffer back to zero-size,
not throw it away and leak whatever was there.


Yes, this one seems to be leaking.

"Next call to the function invalidates the return value the last
caller received" feels like playing with fire.  Most existing
callers are safe in that the first thing they do to the returned
string is xstrdup() it, but we would need to check all the other

That's the price we pay for using static variables, no? Callers need to consume them as long as they're fresh and multi-threading is not allowed. Before, callers could use wrong buffer contents, after the patch they could still have a pointer to freed memory, which should be more noticeable in tests.

Getting a strbuf_add_real_path() in order to avoid static variables would be nice. And it would also be nice if it worked without calling chdir(). Nice topics for follow-up patches. :)

I briefly thought it is not OK for set_git_work_tree(), which gets
new_work_tree, calls real_path() to receive the value from the
function, and then calls real_path() again on it.  The "We've
already done it" optimization is the only thing that makes it safe,
which feels overly fragile.

It wasn't introduced as an optimization, but to silence valgrind (1d679de5: make_absolute_path: return the input path if it points to our buffer). set_git_work_tree() calls real_path() only once in each of its two branches. However, one caller (init) hands it a path returned by real_path(); we can change that (sent a patch).

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to