Skip to content

Commit a71c478

Browse files
peffgitster
authored andcommitted
sparse-checkout: use fdopen_lock_file() instead of xfdopen()
When updating sparse patterns, we open a lock_file to write out the new data. The lock_file struct holds the file descriptor, but we call fdopen() to get a stdio handle to do the actual write. After we finish writing, we fflush() so that all of the data is on disk, and then call commit_lock_file() which closes the descriptor. But we never fclose() the stdio handle, leaking it. The obvious solution seems like it would be to just call fclose(). But when? If we do it before commit_lock_file(), then the lock_file code is left thinking it owns the now-closed file descriptor, and will do an extra close() on the descriptor. But if we do it before, we have the opposite problem: the lock_file code will close the descriptor, and fclose() will do the extra close(). We can handle this correctly by using fdopen_lock_file(). That leaves ownership of the stdio handle with the lock_file, which knows not to double-close it. We do have to adjust the code a bit: - we have to handle errors ourselves; we can just die(), since that's what xfdopen() would have done (and we can even provide a more specific error message). - we no longer need to call fflush(); committing the lock-file auto-closes it, which will now do the flush for us. As a bonus, this will actually check that the flush was successful before renaming the file into place. - we can get rid of the local "fd" variable, since we never look at it ourselves now Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 19ace71 commit a71c478

File tree

1 file changed

+4
-5
lines changed

1 file changed

+4
-5
lines changed

builtin/sparse-checkout.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ static int write_patterns_and_update(struct pattern_list *pl)
327327
{
328328
char *sparse_filename;
329329
FILE *fp;
330-
int fd;
331330
struct lock_file lk = LOCK_INIT;
332331
int result;
333332

@@ -336,8 +335,7 @@ static int write_patterns_and_update(struct pattern_list *pl)
336335
if (safe_create_leading_directories(sparse_filename))
337336
die(_("failed to create directory for sparse-checkout file"));
338337

339-
fd = hold_lock_file_for_update(&lk, sparse_filename,
340-
LOCK_DIE_ON_ERROR);
338+
hold_lock_file_for_update(&lk, sparse_filename, LOCK_DIE_ON_ERROR);
341339

342340
result = update_working_directory(pl);
343341
if (result) {
@@ -346,14 +344,15 @@ static int write_patterns_and_update(struct pattern_list *pl)
346344
goto out;
347345
}
348346

349-
fp = xfdopen(fd, "w");
347+
fp = fdopen_lock_file(&lk, "w");
348+
if (!fp)
349+
die_errno(_("unable to fdopen %s"), get_lock_file_path(&lk));
350350

351351
if (core_sparse_checkout_cone)
352352
write_cone_to_file(fp, pl);
353353
else
354354
write_patterns_to_file(fp, pl);
355355

356-
fflush(fp);
357356
if (commit_lock_file(&lk))
358357
die_errno(_("unable to write %s"), sparse_filename);
359358

0 commit comments

Comments
 (0)