git/tempfile.c, branch jch

path: adjust last remaining users of `the_repository`

2025-02-28T21:54:11Z

With the preceding refactorings we now only have a couple of implicit users of `the_repository` left in the "path" subsystem, all of which depend on global state via `calc_shared_perm()`. Make the dependency on `the_repository` explicit by passing the repo as a parameter instead and adjust callers accordingly. Note that this change bubbles up into a couple of subsystems that were previously declared as free from `the_repository`. Instead of marking all of them as `the_repository`-dependent again, we instead use the repository that is available in the calling context. There are three exceptions though with "copy.c", "pack-write.c" and "tempfile.c". Adjusting these would require us to adapt callsites all over the place, so this is left for a future iteration. Mark "path.c" as free from `the_repository`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

lockfile: report when rollback fails

2024-03-07T20:34:13Z

We do not report to the caller when rolling back a lockfile fails, which will be needed by the reftable compaction logic in a subsequent commit. It also cannot really report on all errors because the function calls `delete_tempfile()`, which doesn't return an error either. Refactor the code so that both `delete_tempfile()` and `rollback_lock_file()` return an error code. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'cw/compat-util-header-cleanup'

2023-07-17T18:30:42Z

Further shuffling of declarations across header files to streamline file dependencies. * cw/compat-util-header-cleanup: git-compat-util: move alloc macros to git-compat-util.h treewide: remove unnecessary includes for wrapper.h kwset: move translation table from ctype sane-ctype.h: create header for sane-ctype macros git-compat-util: move wrapper.c funcs to its header git-compat-util: move strbuf.c funcs to its header

treewide: remove unnecessary includes for wrapper.h

2023-07-05T18:41:59Z

Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano

abspath: move related functions to abspath

2023-06-12T20:49:35Z

Move abspath-related functions from strbuf.[ch] to abspath.[ch] so that strbuf is focused on string manipulation routines with minimal dependencies. Signed-off-by: Calvin Wan Signed-off-by: Junio C Hamano

treewide: remove unnecessary includes of cache.h

2023-03-21T17:56:53Z

The last several commits were geared at replacing the include of cache.h in strbuf.c with an include of git-compat-util.h. Unfortunately, I had to drop a patch moving some functions from cache.h to object-name.h, due to excessive conflicts with other in-flight topics. However, even without that patch, the series of patches so far allows us to modify a number of C files to replace an include of cache.h with git-compat-util.h. Do that to reduce our dependencies. (If we could have kept our object-name.h patch in this series, it would have also let us reduce the includes in checkout.c and fmt-merge-msg.c in addition to strbuf.c). Just to ensure that nothing else was bringing in cache.h, all of the affected files have been checked to ensure that gcc -E -I. $SOURCE_FILE | grep '"cache.h"' found no hits and that make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE} successfully compiles without warnings. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

wrapper.h: move declarations for wrapper.c functions from cache.h

2023-03-21T17:56:53Z

Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

tempfile: update comment describing state transitions

2022-08-30T21:16:51Z

Back when 1a9d15db25 (tempfile: a new module for handling temporary files, 2015-08-10) added this comment, tempfile structs were held in memory for the life of a process, and there were various guarantees about which fields were valid in which states. Since 422a21c6a0 (tempfile: remove deactivated list entries, 2017-09-05) and 076aa2cbda (tempfile: auto-allocate tempfiles on heap, 2017-09-05), the flow is quite different: objects come and go from the list, and inactive ones are deallocated. And the previous commit removed the "active" flag from the struct entirely. Let's bring the comment up to date with the current code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

tempfile: drop active flag

2022-08-30T21:16:49Z

Our tempfile struct contains an "active" flag. Long ago, this flag was important: tempfile structs were always allocated for the lifetime of the program and added to a global linked list, and the active flag was what told us whether a struct's tempfile needed to be cleaned up on exit. But since 422a21c6a0 (tempfile: remove deactivated list entries, 2017-09-05) and 076aa2cbda (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we actually remove items from the list, and the active flag is generally always set to true for any allocated struct. We set it to true in all of the creation functions, and in the normal code flow it becomes false only in deactivate_tempfile(), which then immediately frees the struct. So the flag isn't performing that role anymore, and in fact makes things more confusing. Dscho noted that delete_tempfile() is a noop for an inactive struct. Since 076aa2cbda taught it to free the struct when deactivating, we'd leak any struct whose active flag is unset. But in practice it's not a leak, because again, we'll free when we unset the flag, and never see the allocated-but-inactive state. Can we just get rid of the flag? The answer is yes, but it requires looking at a few other spots: 1. I said above that the flag only becomes false before we deallocate, but there's one exception: when we call remove_tempfiles() from a signal or atexit handler, we unset the active flag as we remove each file. This isn't important for delete_tempfile(), as nobody would call it anymore, since we're exiting. It does in theory provide us some protection against racily double-removing a tempfile. If we receive a second signal while we are already in the cleanup routines, we'll start the cleanup loop again, and may visit the same tempfile. But this race already exists, because calling unlink() and unsetting the active flag aren't atomic! And it's OK in practice, because unlink() is idempotent (barring the unlikely event that some other process chooses our exact temp filename in that instant). So dropping the active flag widens the race a bit, but it was already there, and is fairly harmless in practice. If we really care about addressing it, the right thing is probably to block further signals while we're doing our cleanup (which we could actually do atomically). 2. The active flag is declared as "volatile sig_atomic_t". The idea is that it's the final bit that gets set to tell the cleanup routines that the tempfile is ready to be used (or not used), and it's safe to receive a signal racing with regular code which adds or removes a tempfile from the list. In practice, I don't think this is buying us anything. The presence on the linked list is really what tells the cleanup routines to look at the struct. That is already marked as "volatile". It's not a sig_atomic_t, so it's possible that we could see a sheared write there as an entry is added or removed. But that is true of the current code, too! Before we can even look at the "active" flag, we'd have to follow a link to the struct itself. If we see a sheared write in the pointer to the struct, then we'll look at garbage memory anyway, and there's not much we can do. This patch removes the active flag entirely, using presence on the global linked list as an indicator that a tempfile ought to be cleaned up. We are already careful to add to the list as the final step in activating. On deactivation, we'll make sure to remove from the list as the first step, before freeing any fields. The use of the volatile keyword should mean that those things happen in the expected order. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

tempfile: avoid directory cleanup race

2022-08-27T17:17:46Z

The temporary directory created by mks_tempfile_dt() is deleted by first deleting the file within, then truncating the filename strbuf and passing the resulting string to rmdir(2). When the cleanup routine is invoked concurrently by a signal handler we can end up passing the now truncated string to unlink(2), however, which could cause problems on some systems. Avoid that issue by remembering the directory name separately. This way the paths stay unchanged. A signal handler can still race with normal cleanup, but deleting the same files and directories twice is harmless. Reported-by: Jeff King Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano