@matheusbsbs / matheus-bagliano / commits / 4ea82473aa

link_alt_odb_entry: refactor string handling

The string handling in link_alt_odb_entry() is mostly an artifact of the original version, which took the path as a ptr/len combo, and did not have a NUL-terminated string until we created one in the alternate_object_database struct. But since 5bdf0a8 (sha1_file: normalize alt_odb path before comparing and storing, 2011-09-07), the first thing we do is put the path into a strbuf, which gives us some easy opportunities for cleanup. In particular: - we call strlen(pathbuf.buf), which is silly; we can look at pathbuf.len. - even though we have a strbuf, we don't maintain its "len" field when chomping extra slashes from the end, and instead keep a separate "pfxlen" variable. We can fix this and then drop "pfxlen" entirely. - we don't check whether the path is usable until after we allocate the new struct, making extra cleanup work for ourselves. Since we have a NUL-terminated string, we can bump the "is it usable" checks higher in the function. While we're at it, we can move that logic to its own helper, which makes the flow of link_alt_odb_entry() easier to follow. 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:34 UTC 4ea82473aa310de7543141f96c2e6b23ef9fcd4c

1 file changed +45 -38

sha1_file.c

+45 -38

@@ -234,6 +234,36 @@ char *sha1_pack_index_name(const unsigned char *sha1)

234 struct alternate_object_database *alt_odb_list;

235 static struct alternate_object_database **alt_odb_tail;

236

237 +/*

238 + * Return non-zero iff the path is usable as an alternate object database.

239 + */

240 +static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)

241 +{

242 + struct alternate_object_database *alt;

243 +

244 + /* Detect cases where alternate disappeared */

245 + if (!is_directory(path->buf)) {

246 + error("object directory %s does not exist; "

247 + "check .git/objects/info/alternates.",

248 + path->buf);

249 + return 0;

250 + }

251 +

252 + /*

253 + * Prevent the common mistake of listing the same

254 + * thing twice, or object directory itself.

255 + */

256 + for (alt = alt_odb_list; alt; alt = alt->next) {

257 + if (path->len == alt->name - alt->base - 1 &&

258 + !memcmp(path->buf, alt->base, path->len))

259 + return 0;

260 + }

261 + if (!fspathcmp(path->buf, normalized_objdir))

262 + return 0;

263 +

264 + return 1;

265 +}

266 +

267 /*

268 * Prepare alternate object database registry.

269 *

@@ -253,8 +283,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,

283 int depth, const char *normalized_objdir)

284 {

285 struct alternate_object_database *ent;

256 - struct alternate_object_database *alt;

257 - size_t pfxlen, entlen;

286 + size_t entlen;

287 struct strbuf pathbuf = STRBUF_INIT;

288

289 if (!is_absolute_path(entry) && relative_base) {

@@ -270,47 +299,26 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,

299 return -1;

300 }

301

273 - pfxlen = strlen(pathbuf.buf);

274 -

302 /*

303 * The trailing slash after the directory name is given by

304 * this function at the end. Remove duplicates.

305 */

279 - while (pfxlen && pathbuf.buf[pfxlen-1] == '/')

280 - pfxlen -= 1;

281 -

282 - entlen = st_add(pfxlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */

283 - ent = xmalloc(st_add(sizeof(*ent), entlen));

284 - memcpy(ent->base, pathbuf.buf, pfxlen);

285 - strbuf_release(&pathbuf);

286 -

287 - ent->name = ent->base + pfxlen + 1;

288 - ent->base[pfxlen + 3] = '/';

289 - ent->base[pfxlen] = ent->base[entlen-1] = 0;

306 + while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')

307 + strbuf_setlen(&pathbuf, pathbuf.len - 1);

308

291 - /* Detect cases where alternate disappeared */

292 - if (!is_directory(ent->base)) {

293 - error("object directory %s does not exist; "

294 - "check .git/objects/info/alternates.",

295 - ent->base);

296 - free(ent);

309 + if (!alt_odb_usable(&pathbuf, normalized_objdir)) {

310 + strbuf_release(&pathbuf);

311 return -1;

312 }

313

300 - /* Prevent the common mistake of listing the same

301 - * thing twice, or object directory itself.

302 - */

303 - for (alt = alt_odb_list; alt; alt = alt->next) {

304 - if (pfxlen == alt->name - alt->base - 1 &&

305 - !memcmp(ent->base, alt->base, pfxlen)) {

306 - free(ent);

307 - return -1;

308 - }

309 - }

310 - if (!fspathcmp(ent->base, normalized_objdir)) {

311 - free(ent);

312 - return -1;

313 - }

314 + entlen = st_add(pathbuf.len, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */

315 + ent = xmalloc(st_add(sizeof(*ent), entlen));

316 + memcpy(ent->base, pathbuf.buf, pathbuf.len);

317 +

318 + ent->name = ent->base + pathbuf.len + 1;

319 + ent->base[pathbuf.len] = '/';

320 + ent->base[pathbuf.len + 3] = '/';

321 + ent->base[entlen-1] = 0;

322

323 /* add the alternate entry */

324 *alt_odb_tail = ent;

@@ -318,10 +326,9 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,

326 ent->next = NULL;

327

328 /* recursively add alternates */

321 - read_info_alternates(ent->base, depth + 1);

322 -

323 - ent->base[pfxlen] = '/';

329 + read_info_alternates(pathbuf.buf, depth + 1);

330

331 + strbuf_release(&pathbuf);

332 return 0;

333 }

334