Web lists-archives.com

Re: [PATCH] sha1_file: add slash once in for_each_file_in_obj_subdir()




Am 09.07.2017 um 13:00 schrieb Jeff King:
On Sat, Jul 08, 2017 at 10:59:06AM +0200, René Scharfe wrote:

Add the slash between loose object subdirectory and file name just once
outside the loop instead of overwriting it with each readdir call.
Redefine baselen as the length with that slash, and add dirlen for the
length without it.  The result is slightly less wasteful and can use the
the cheaper strbuf_addstr instead of strbuf_addf without losing clarity.

This patch looks correct to me.

I'm a little lukewarm on it overall, though. I'd be shocked if the
efficiency change is measurable. What I really care about is whether the
result is easier to read or not.

On the plus side, this moves an invariant out of the loop. On the minus
side, it has to introduce an extra variable for "length we add on to"
versus "dir length to pass to the subdir_cb". That's not rocket science,
but it does slightly complicate things (though I note we already have
"origlen", so this is bumping us from 2 to 3 length variables, not 1 to
2).

Admittedly this is more of an OCD thing.  Overwriting a character 200
times or parsing a format string don't very long compared to the rest
of the function, but it's a bit itchy.

René