From: Junio C Hamano Date: Tue, 19 May 2015 20:17:59 +0000 (-0700) Subject: Merge branch 'sb/ref-lock-lose-lock-fd' X-Git-Tag: v2.5.0-rc0~104 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/4295abc04045c0bd2160bc7f6a4971abdcabf3b8?ds=inline;hp=-c Merge branch 'sb/ref-lock-lose-lock-fd' The refs API uses ref_lock struct which had its own "int fd", even though the same file descriptor was in the lock struct it contains. Clean-up the code to lose this redundant field. * sb/ref-lock-lose-lock-fd: refs.c: remove lock_fd from struct ref_lock --- 4295abc04045c0bd2160bc7f6a4971abdcabf3b8 diff --combined refs.c index 81a455b807,4f495bd762..b5189f4a57 --- a/refs.c +++ b/refs.c @@@ -11,7 -11,6 +11,6 @@@ struct ref_lock char *orig_ref_name; struct lock_file *lk; unsigned char old_sha1[20]; - int lock_fd; }; /* @@@ -344,6 -343,8 +343,6 @@@ static struct ref_entry *create_ref_ent if (check_name && check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) die("Reference has invalid format: '%s'", refname); - if (!check_name && !refname_is_safe(refname)) - die("Reference has invalid name: '%s'", refname); len = strlen(refname) + 1; ref = xmalloc(sizeof(struct ref_entry) + len); hashcpy(ref->u.value.sha1, sha1); @@@ -1176,8 -1177,6 +1175,8 @@@ static void read_packed_refs(FILE *f, s int flag = REF_ISPACKED; if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { + if (!refname_is_safe(refname)) + die("packed refname is dangerous: %s", refname); hashclr(sha1); flag |= REF_BAD_NAME | REF_ISBROKEN; } @@@ -1323,8 -1322,6 +1322,8 @@@ static void read_loose_refs(const char } if (check_refname_format(refname.buf, REFNAME_ALLOW_ONELEVEL)) { + if (!refname_is_safe(refname.buf)) + die("loose refname is dangerous: %s", refname.buf); hashclr(sha1); flag |= REF_BAD_NAME | REF_ISBROKEN; } @@@ -1384,7 -1381,7 +1383,7 @@@ static int resolve_gitlink_ref_recursiv { int fd, len; char buffer[128], *p; - char *path; + const char *path; if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN) return -1; @@@ -1477,11 -1474,7 +1476,11 @@@ static int resolve_missing_loose_ref(co } /* This function needs to return a meaningful errno on failure */ -const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned char *sha1, int *flags) +static const char *resolve_ref_unsafe_1(const char *refname, + int resolve_flags, + unsigned char *sha1, + int *flags, + struct strbuf *sb_path) { int depth = MAXDEPTH; ssize_t len; @@@ -1512,7 -1505,7 +1511,7 @@@ bad_name = 1; } for (;;) { - char path[PATH_MAX]; + const char *path; struct stat st; char *buf; int fd; @@@ -1522,9 -1515,7 +1521,9 @@@ return NULL; } - git_snpath(path, sizeof(path), "%s", refname); + strbuf_reset(sb_path); + strbuf_git_path(sb_path, "%s", refname); + path = sb_path->buf; /* * We might have to loop back here to avoid a race @@@ -1651,16 -1642,6 +1650,16 @@@ } } +const char *resolve_ref_unsafe(const char *refname, int resolve_flags, + unsigned char *sha1, int *flags) +{ + struct strbuf sb_path = STRBUF_INIT; + const char *ret = resolve_ref_unsafe_1(refname, resolve_flags, + sha1, flags, &sb_path); + strbuf_release(&sb_path); + return ret; +} + char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags) { return xstrdup_or_null(resolve_ref_unsafe(ref, resolve_flags, sha1, flags)); @@@ -2292,7 -2273,7 +2291,7 @@@ static struct ref_lock *lock_ref_sha1_b const struct string_list *skip, unsigned int flags, int *type_p) { - char *ref_file; + const char *ref_file; const char *orig_refname = refname; struct ref_lock *lock; int last_errno = 0; @@@ -2302,7 -2283,6 +2301,6 @@@ int attempts_remaining = 3; lock = xcalloc(1, sizeof(struct ref_lock)); - lock->lock_fd = -1; if (mustexist) resolve_flags |= RESOLVE_REF_READING; @@@ -2361,7 -2341,7 +2359,7 @@@ ref_file = git_path("%s", refname); retry: - switch (safe_create_leading_directories(ref_file)) { + switch (safe_create_leading_directories_const(ref_file)) { case SCLD_OK: break; /* success */ case SCLD_VANISHED: @@@ -2374,8 -2354,7 +2372,7 @@@ goto error_return; } - lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags); - if (lock->lock_fd < 0) { + if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) { last_errno = errno; if (errno == ENOENT && --attempts_remaining > 0) /* @@@ -2739,7 -2718,7 +2736,7 @@@ static int rename_tmp_log(const char *n int attempts_remaining = 4; retry: - switch (safe_create_leading_directories(git_path("logs/%s", newrefname))) { + switch (safe_create_leading_directories_const(git_path("logs/%s", newrefname))) { case SCLD_OK: break; /* success */ case SCLD_VANISHED: @@@ -2886,7 -2865,6 +2883,6 @@@ static int close_ref(struct ref_lock *l { if (close_lock_file(lock->lk)) return -1; - lock->lock_fd = -1; return 0; } @@@ -2894,7 -2872,6 +2890,6 @@@ static int commit_ref(struct ref_lock * { if (commit_lock_file(lock->lk)) return -1; - lock->lock_fd = -1; return 0; } @@@ -2925,15 -2902,11 +2920,15 @@@ static int copy_msg(char *buf, const ch } /* This function must set a meaningful errno on failure */ -int log_ref_setup(const char *refname, char *logfile, int bufsize) +int log_ref_setup(const char *refname, struct strbuf *sb_logfile) { int logfd, oflags = O_APPEND | O_WRONLY; + char *logfile; - git_snpath(logfile, bufsize, "logs/%s", refname); + strbuf_git_path(sb_logfile, "logs/%s", refname); + logfile = sb_logfile->buf; + /* make sure the rest of the function can't change "logfile" */ + sb_logfile = NULL; if (log_all_ref_updates && (starts_with(refname, "refs/heads/") || starts_with(refname, "refs/remotes/") || @@@ -3004,22 -2977,18 +2999,22 @@@ static int log_ref_write_fd(int fd, con return 0; } -static int log_ref_write(const char *refname, const unsigned char *old_sha1, - const unsigned char *new_sha1, const char *msg) +static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, + const unsigned char *new_sha1, const char *msg, + struct strbuf *sb_log_file) { int logfd, result, oflags = O_APPEND | O_WRONLY; - char log_file[PATH_MAX]; + char *log_file; if (log_all_ref_updates < 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, log_file, sizeof(log_file)); + result = log_ref_setup(refname, sb_log_file); if (result) return result; + log_file = sb_log_file->buf; + /* make sure the rest of the function can't change "log_file" */ + sb_log_file = NULL; logfd = open(log_file, oflags); if (logfd < 0) @@@ -3042,15 -3011,6 +3037,15 @@@ return 0; } +static int log_ref_write(const char *refname, const unsigned char *old_sha1, + const unsigned char *new_sha1, const char *msg) +{ + struct strbuf sb = STRBUF_INIT; + int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb); + strbuf_release(&sb); + return ret; +} + int is_branch(const char *refname) { return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/"); @@@ -3081,8 -3041,8 +3076,8 @@@ static int write_ref_sha1(struct ref_lo errno = EINVAL; return -1; } - if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 || - write_in_full(lock->lock_fd, &term, 1) != 1 || + if (write_in_full(lock->lk->fd, sha1_to_hex(sha1), 40) != 40 || + write_in_full(lock->lk->fd, &term, 1) != 1 || close_ref(lock) < 0) { int save_errno = errno; error("Couldn't write %s", lock->lk->filename.buf); @@@ -4119,9 -4079,9 +4114,9 @@@ int reflog_expire(const char *refname, status |= error("couldn't write %s: %s", log_file, strerror(errno)); } else if (update && - (write_in_full(lock->lock_fd, + (write_in_full(lock->lk->fd, sha1_to_hex(cb.last_kept_sha1), 40) != 40 || - write_str_in_full(lock->lock_fd, "\n") != 1 || + write_str_in_full(lock->lk->fd, "\n") != 1 || close_ref(lock) < 0)) { status |= error("couldn't write %s", lock->lk->filename.buf);