Am 20.07.2014 14:33, schrieb Duy Nguyen:
On Sun, Jul 20, 2014 at 6:21 PM, René Scharfe l@web.de wrote:
+int strbuf_add_cwd(struct strbuf *sb)
+{
+ size_t oldalloc = sb-alloc;
+ size_t guessed_len = 32;
For Linux, I think this is enough to succesfully get cwd in the first
pass. Windows' $HOME is usually deep in C:\Users\Blahblah. Maybe
increase this to 128? And we probably want to keep the guessed value,
so if the first strbuf_add_cwd needs a few rounds to get cwd, the next
strbuf_add_cwd() call does not.
The initial number is debatable, sure. I just copied the 32 from
strbuf_readline().
C:\Users\John Doe\Documents\Projects\foo (or whatever) isn't thaaat
long either, though. FWIW, the longest (non-hidden) path in my home on
Windows 8.1 is 139 characters long (as reported by dir -r | %{
$_.FullName.Length } | sort -desc | select -f 1), but there are no git
projects in that directory. Not sure if that means 128 would be a
better start value, but I don't mind changing it in any case.
Before adding performance optimizations like remembering the last cwd
length I'd rather see measurements that demonstrate their use. I doubt
we'll see getcwd() show up high on a profile (but didn't measure, except
for a run of t/perf).
+
+ for (;; guessed_len *= 2) {
+ char *cwd;
+
+ strbuf_grow(sb, guessed_len);
+ cwd = getcwd(sb-buf + sb-len, sb-alloc - sb-len);
+ if (cwd) {
+ strbuf_setlen(sb, sb-len + strlen(cwd));
+ return 0;
+ }
+ if (errno != ERANGE)
+ break;
+ }
+ if (oldalloc == 0)
+ strbuf_release(sb);
+ return -1;
+}
The loop and the following strbuf_release() are correct. But I wonder
if it's easier to read if you save getcwd in a separate/local strbuf
variable and only concat it to sb when you successfully get cwd..
Adding an extra allocation and string copy sound more complicated. But
you are right that the function is more complicated than necessary.
Reroll coming..
René
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
