refs.c: avoid git_path assignment in lock_ref_sha1_basic
Assigning the result of git_path is a bad pattern, because it's not immediately obvious how long you expect the content to stay valid (and it may be overwritten by subsequent calls). Let's use a function-local strbuf here instead, which we know is safe (we just have to remember to free it in all code paths). As a bonus, we get rid of a confusing variable-reuse ("ref_file" is used for two distinct purposes). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Aug 10, 2015 at 05:37 UTC
5f8ef5b84889e7792e929a0fc773cb0060a0a611
1 file changed
+19
-13
refs.c
+19
-13
@@ -2408,7 +2408,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2408
unsigned int flags, int *type_p,
2409
struct strbuf *err)
2410
{
2411
- const char *ref_file;
2411
+ struct strbuf ref_file = STRBUF_INIT;
2412
+ struct strbuf orig_ref_file = STRBUF_INIT;
2413
const char *orig_refname = refname;
2414
struct ref_lock *lock;
2415
int last_errno = 0;
@@ -2432,20 +2433,19 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2433
refname = resolve_ref_unsafe(refname, resolve_flags,
2434
lock->old_oid.hash, &type);
2435
if (!refname && errno == EISDIR) {
2435
- /* we are trying to lock foo but we used to
2436
+ /*
2437
+ * we are trying to lock foo but we used to
2438
* have foo/bar which now does not exist;
2439
* it is normal for the empty directory 'foo'
2440
* to remain.
2441
*/
2440
- ref_file = git_path("%s", orig_refname);
2441
- if (remove_empty_directories(ref_file)) {
2442
+ strbuf_git_path(&orig_ref_file, "%s", orig_refname);
2443
+ if (remove_empty_directories(orig_ref_file.buf)) {
2444
last_errno = errno;
2443
-
2445
if (!verify_refname_available(orig_refname, extras, skip,
2446
get_loose_refs(&ref_cache), err))
2447
strbuf_addf(err, "there are still refs under '%s'",
2448
orig_refname);
2448
-
2449
goto error_return;
2450
}
2451
refname = resolve_ref_unsafe(orig_refname, resolve_flags,
@@ -2485,10 +2485,10 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2485
}
2486
lock->ref_name = xstrdup(refname);
2487
lock->orig_ref_name = xstrdup(orig_refname);
2488
- ref_file = git_path("%s", refname);
2488
+ strbuf_git_path(&ref_file, "%s", refname);
2489
2490
retry:
2491
- switch (safe_create_leading_directories_const(ref_file)) {
2491
+ switch (safe_create_leading_directories_const(ref_file.buf)) {
2492
case SCLD_OK:
2493
break; /* success */
2494
case SCLD_VANISHED:
@@ -2497,11 +2497,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2497
/* fall through */
2498
default:
2499
last_errno = errno;
2500
- strbuf_addf(err, "unable to create directory for %s", ref_file);
2500
+ strbuf_addf(err, "unable to create directory for %s",
2501
+ ref_file.buf);
2502
goto error_return;
2503
}
2504
2504
- if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
2505
+ if (hold_lock_file_for_update(lock->lk, ref_file.buf, lflags) < 0) {
2506
last_errno = errno;
2507
if (errno == ENOENT && --attempts_remaining > 0)
2508
/*
@@ -2511,7 +2512,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2512
*/
2513
goto retry;
2514
else {
2514
- unable_to_lock_message(ref_file, errno, err);
2515
+ unable_to_lock_message(ref_file.buf, errno, err);
2516
goto error_return;
2517
}
2518
}
@@ -2519,12 +2520,17 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2520
last_errno = errno;
2521
goto error_return;
2522
}
2522
- return lock;
2523
+ goto out;
2524
2525
error_return:
2526
unlock_ref(lock);
2527
+ lock = NULL;
2528
+
2529
+ out:
2530
+ strbuf_release(&ref_file);
2531
+ strbuf_release(&orig_ref_file);
2532
errno = last_errno;
2527
- return NULL;
2533
+ return lock;
2534
}
2535
2536
/*