Skip to content

Commit 1fb0c80

Browse files
mhaggergitster
authored andcommitted
log_ref_setup(): improve robustness against races
Change log_ref_setup() to use raceproof_create_file() to create the new logfile. This makes it more robust against a race against another process that might be trying to clean up empty directories while we are trying to create a new logfile. This also means that it will only call create_leading_directories() if open() fails, which should be a net win. Even in the cases where we are willing to create a new logfile, it will usually be the case that the logfile already exists, or if not then that the directory containing the logfile already exists. In such cases, we will save some work that was previously done unconditionally. Signed-off-by: Michael Haggerty <[email protected]> Reviewed-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 854bda6 commit 1fb0c80

File tree

1 file changed

+18
-23
lines changed

1 file changed

+18
-23
lines changed

refs/files-backend.c

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2710,6 +2710,14 @@ static int commit_ref(struct ref_lock *lock)
27102710
return 0;
27112711
}
27122712

2713+
static int open_or_create_logfile(const char *path, void *cb)
2714+
{
2715+
int *fd = cb;
2716+
2717+
*fd = open(path, O_APPEND | O_WRONLY | O_CREAT, 0666);
2718+
return (*fd < 0) ? -1 : 0;
2719+
}
2720+
27132721
/*
27142722
* Create a reflog for a ref. If force_create = 0, the reflog will
27152723
* only be created for certain refs (those for which
@@ -2723,31 +2731,18 @@ static int log_ref_setup(const char *refname, struct strbuf *logfile, struct str
27232731
strbuf_git_path(logfile, "logs/%s", refname);
27242732

27252733
if (force_create || should_autocreate_reflog(refname)) {
2726-
if (safe_create_leading_directories(logfile->buf) < 0) {
2727-
strbuf_addf(err, "unable to create directory for '%s': "
2728-
"%s", logfile->buf, strerror(errno));
2729-
return -1;
2730-
}
2731-
logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
2732-
if (logfd < 0) {
2733-
if (errno == EISDIR) {
2734-
/*
2735-
* The directory that is in the way might be
2736-
* empty. Try to remove it.
2737-
*/
2738-
if (remove_empty_directories(logfile)) {
2739-
strbuf_addf(err, "there are still logs under "
2740-
"'%s'", logfile->buf);
2741-
return -1;
2742-
}
2743-
logfd = open(logfile->buf, O_APPEND | O_WRONLY | O_CREAT, 0666);
2744-
}
2745-
2746-
if (logfd < 0) {
2734+
if (raceproof_create_file(logfile->buf, open_or_create_logfile, &logfd)) {
2735+
if (errno == ENOENT)
2736+
strbuf_addf(err, "unable to create directory for '%s': "
2737+
"%s", logfile->buf, strerror(errno));
2738+
else if (errno == EISDIR)
2739+
strbuf_addf(err, "there are still logs under '%s'",
2740+
logfile->buf);
2741+
else
27472742
strbuf_addf(err, "unable to append to '%s': %s",
27482743
logfile->buf, strerror(errno));
2749-
return -1;
2750-
}
2744+
2745+
return -1;
27512746
}
27522747
} else {
27532748
logfd = open(logfile->buf, O_APPEND | O_WRONLY, 0666);

0 commit comments

Comments
 (0)