read_raw_ref(): manage own scratch space

Instead of creating scratch space in resolve_ref_unsafe() and passing it down through resolve_ref_1 to read_raw_ref(), teach read_raw_ref() to manage its own scratch space. This reduces coupling across the functions at the cost of some extra allocations. Also, when read_raw_ref() is implemented for different reference backends, the other implementations might have different scratch space requirements. Note that we now preserve errno across the calls to strbuf_release(), which calls free() and can thus theoretically overwrite errno. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Apr 7, 2016 at 15:03 UTC 42a38cf788428bf39db1729ee8c79c64252a2849
1 file changed +41 -35
refs/files-backend.c
+41 -35
@@ -1421,17 +1421,20 @@ static int resolve_missing_loose_ref(const char *refname,
1421 * refname will still be valid and unchanged.
1422 */
1423 static int read_raw_ref(const char *refname, unsigned char *sha1,
1424 - struct strbuf *symref, struct strbuf *sb_path,
1425 - struct strbuf *sb_contents, int *flags)
1424 + struct strbuf *symref, int *flags)
1425 {
1426 + struct strbuf sb_contents = STRBUF_INIT;
1427 + struct strbuf sb_path = STRBUF_INIT;
1428 const char *path;
1429 const char *buf;
1430 struct stat st;
1431 int fd;
1432 + int ret = -1;
1433 + int save_errno;
1434
1432 - strbuf_reset(sb_path);
1433 - strbuf_git_path(sb_path, "%s", refname);
1434 - path = sb_path->buf;
1435 + strbuf_reset(&sb_path);
1436 + strbuf_git_path(&sb_path, "%s", refname);
1437 + path = sb_path.buf;
1438
1439 stat_ref:
1440 /*
@@ -1446,36 +1449,38 @@ stat_ref:
1449
1450 if (lstat(path, &st) < 0) {
1451 if (errno != ENOENT)
1449 - return -1;
1452 + goto out;
1453 if (resolve_missing_loose_ref(refname, sha1, flags)) {
1454 errno = ENOENT;
1452 - return -1;
1455 + goto out;
1456 }
1454 - return 0;
1457 + ret = 0;
1458 + goto out;
1459 }
1460
1461 /* Follow "normalized" - ie "refs/.." symlinks by hand */
1462 if (S_ISLNK(st.st_mode)) {
1459 - strbuf_reset(sb_contents);
1460 - if (strbuf_readlink(sb_contents, path, 0) < 0) {
1463 + strbuf_reset(&sb_contents);
1464 + if (strbuf_readlink(&sb_contents, path, 0) < 0) {
1465 if (errno == ENOENT || errno == EINVAL)
1466 /* inconsistent with lstat; retry */
1467 goto stat_ref;
1468 else
1465 - return -1;
1469 + goto out;
1470 }
1467 - if (starts_with(sb_contents->buf, "refs/") &&
1468 - !check_refname_format(sb_contents->buf, 0)) {
1469 - strbuf_swap(sb_contents, symref);
1471 + if (starts_with(sb_contents.buf, "refs/") &&
1472 + !check_refname_format(sb_contents.buf, 0)) {
1473 + strbuf_swap(&sb_contents, symref);
1474 *flags |= REF_ISSYMREF;
1471 - return 0;
1475 + ret = 0;
1476 + goto out;
1477 }
1478 }
1479
1480 /* Is it a directory? */
1481 if (S_ISDIR(st.st_mode)) {
1482 errno = EISDIR;
1478 - return -1;
1483 + goto out;
1484 }
1485
1486 /*
@@ -1488,18 +1493,18 @@ stat_ref:
1493 /* inconsistent with lstat; retry */
1494 goto stat_ref;
1495 else
1491 - return -1;
1496 + goto out;
1497 }
1493 - strbuf_reset(sb_contents);
1494 - if (strbuf_read(sb_contents, fd, 256) < 0) {
1498 + strbuf_reset(&sb_contents);
1499 + if (strbuf_read(&sb_contents, fd, 256) < 0) {
1500 int save_errno = errno;
1501 close(fd);
1502 errno = save_errno;
1498 - return -1;
1503 + goto out;
1504 }
1505 close(fd);
1501 - strbuf_rtrim(sb_contents);
1502 - buf = sb_contents->buf;
1506 + strbuf_rtrim(&sb_contents);
1507 + buf = sb_contents.buf;
1508 if (starts_with(buf, "ref:")) {
1509 buf += 4;
1510 while (isspace(*buf))
@@ -1508,7 +1513,8 @@ stat_ref:
1513 strbuf_reset(symref);
1514 strbuf_addstr(symref, buf);
1515 *flags |= REF_ISSYMREF;
1511 - return 0;
1516 + ret = 0;
1517 + goto out;
1518 }
1519
1520 /*
@@ -1519,10 +1525,17 @@ stat_ref:
1525 (buf[40] != '\0' && !isspace(buf[40]))) {
1526 *flags |= REF_ISBROKEN;
1527 errno = EINVAL;
1522 - return -1;
1528 + goto out;
1529 }
1530
1525 - return 0;
1531 + ret = 0;
1532 +
1533 +out:
1534 + save_errno = errno;
1535 + strbuf_release(&sb_path);
1536 + strbuf_release(&sb_contents);
1537 + errno = save_errno;
1538 + return ret;
1539 }
1540
1541 /* This function needs to return a meaningful errno on failure */
@@ -1530,9 +1543,7 @@ static const char *resolve_ref_1(const char *refname,
1543 int resolve_flags,
1544 unsigned char *sha1,
1545 int *flags,
1533 - struct strbuf *sb_refname,
1534 - struct strbuf *sb_path,
1535 - struct strbuf *sb_contents)
1546 + struct strbuf *sb_refname)
1547 {
1548 int symref_count;
1549
@@ -1559,8 +1570,7 @@ static const char *resolve_ref_1(const char *refname,
1570 for (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {
1571 int read_flags = 0;
1572
1562 - if (read_raw_ref(refname, sha1, sb_refname,
1563 - sb_path, sb_contents, &read_flags)) {
1573 + if (read_raw_ref(refname, sha1, sb_refname, &read_flags)) {
1574 *flags |= read_flags;
1575 if (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))
1576 return NULL;
@@ -1604,8 +1614,6 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
1614 unsigned char *sha1, int *flags)
1615 {
1616 static struct strbuf sb_refname = STRBUF_INIT;
1607 - struct strbuf sb_contents = STRBUF_INIT;
1608 - struct strbuf sb_path = STRBUF_INIT;
1617 int unused_flags;
1618 const char *ret;
1619
@@ -1613,9 +1621,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
1621 flags = &unused_flags;
1622
1623 ret = resolve_ref_1(refname, resolve_flags, sha1, flags,
1616 - &sb_refname, &sb_path, &sb_contents);
1617 - strbuf_release(&sb_path);
1618 - strbuf_release(&sb_contents);
1624 + &sb_refname);
1625 return ret;
1626 }
1627