@matheusbsbs / matheus-bagliano / commits / f7b7774f34

fill_sha1_file: write into a strbuf

It's currently the responsibility of the caller to give fill_sha1_file() enough bytes to write into, leading them to manually compute the required lengths. Instead, let's just write into a strbuf so that it's impossible to get this wrong. The alt_odb caller already has a strbuf, so this makes things strictly simpler. The other caller, sha1_file_name(), uses a static PATH_MAX buffer and dies when it would overflow. We can convert this to a static strbuf, which means our allocation cost is amortized (and as a bonus, we no longer have to worry about PATH_MAX being too short for normal use). This does introduce some small overhead in fill_sha1_file(), as each strbuf_addchar() will check whether it needs to grow. However, between the optimization in fec501d (strbuf_addch: avoid calling strbuf_grow, 2015-04-16) and the fact that this is not generally called in a tight loop (after all, the next step is typically to access the file!) this probably doesn't matter. And even if it did, the right place to micro-optimize is inside fill_sha1_file(), by calling a single strbuf_grow() there. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 3, 2016 at 16:36 UTC f7b7774f34b86fed8a2e9554a9fe865c62a0a5eb

1 file changed +10 -24

sha1_file.c

+10 -24

@@ -172,36 +172,28 @@ enum scld_error safe_create_leading_directories_const(const char *path)

172 return result;

173 }

174

175 -static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)

175 +static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)

176 {

177 int i;

178 for (i = 0; i < 20; i++) {

179 static char hex[] = "0123456789abcdef";

180 unsigned int val = sha1[i];

181 - *pathbuf++ = hex[val >> 4];

182 - *pathbuf++ = hex[val & 0xf];

181 + strbuf_addch(buf, hex[val >> 4]);

182 + strbuf_addch(buf, hex[val & 0xf]);

183 if (!i)

184 - *pathbuf++ = '/';

184 + strbuf_addch(buf, '/');

185 }

186 - *pathbuf = '\0';

186 }

187

188 const char *sha1_file_name(const unsigned char *sha1)

189 {

191 - static char buf[PATH_MAX];

192 - const char *objdir;

193 - int len;

190 + static struct strbuf buf = STRBUF_INIT;

191

195 - objdir = get_object_directory();

196 - len = strlen(objdir);

192 + strbuf_reset(&buf);

193 + strbuf_addf(&buf, "%s/", get_object_directory());

194

198 - /* '/' + sha1(2) + '/' + sha1(38) + '\0' */

199 - if (len + 43 > PATH_MAX)

200 - die("insanely long object directory %s", objdir);

201 - memcpy(buf, objdir, len);

202 - buf[len] = '/';

203 - fill_sha1_path(buf + len + 1, sha1);

204 - return buf;

195 + fill_sha1_path(&buf, sha1);

196 + return buf.buf;

197 }

198

199 struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)

@@ -213,14 +205,8 @@ struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)

205 static const char *alt_sha1_path(struct alternate_object_database *alt,

206 const unsigned char *sha1)

207 {

216 - /* hex sha1 plus internal "/" */

217 - size_t len = GIT_SHA1_HEXSZ + 1;

208 struct strbuf *buf = alt_scratch_buf(alt);

219 -

220 - strbuf_grow(buf, len);

221 - fill_sha1_path(buf->buf + buf->len, sha1);

222 - strbuf_setlen(buf, buf->len + len);

223 -

209 + fill_sha1_path(buf, sha1);

210 return buf->buf;

211 }

212