The function remove_lock_file_on_signal() is used as a signal handler.
It is not realistic to make the signal handler conform strictly to the
C standard, which is very restrictive about what a signal handler is
allowed to do. But let's increase the likelihood that it will work:
The lock_file_list global variable and several fields from struct
lock_file are used by the signal handler. Declare those values
"volatile" to (1) force the main process to write the values to RAM
promptly, and (2) prevent updates to these fields from being reordered
in a way that leaves an opportunity for a jump to the signal handler
while the object is in an inconsistent state.
We don't mark the filename field volatile because that would prevent
the use of strcpy(), and it is anyway unlikely that a compiler
re-orders a strcpy() call across other expressions. So in practice it
should be possible to get away without "volatile" in the "filename"
case.
Suggested-by: Johannes Sixt <j6t@xxxxxxxx>
Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
---
cache.h | 6 +++---
lockfile.c | 2 +-
refs.c | 5 +++--
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/cache.h b/cache.h
index 81c70c7..0e55bbe 100644
--- a/cache.h
+++ b/cache.h
@@ -575,10 +575,10 @@ extern int refresh_index(struct index_state *, unsigned int flags, const struct
#define LOCK_SUFFIX_LEN 5
struct lock_file {
- struct lock_file *next;
+ struct lock_file *volatile next;
volatile sig_atomic_t active;
- int fd;
- pid_t owner;
+ volatile int fd;
+ volatile pid_t owner;
char on_list;
char filename[PATH_MAX];
};
diff --git a/lockfile.c b/lockfile.c
index d35ac44..89043f5 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -55,7 +55,7 @@
* on_list is set.
*/
-static struct lock_file *lock_file_list;
+static struct lock_file *volatile lock_file_list;
static void remove_lock_file(void)
{
diff --git a/refs.c b/refs.c
index 4f313bc..9971ac5 100644
--- a/refs.c
+++ b/refs.c
@@ -2259,15 +2259,16 @@ int commit_packed_refs(void)
get_packed_ref_cache(&ref_cache);
int error = 0;
int save_errno = 0;
+ int fd;
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
write_or_die(packed_ref_cache->lock->fd,
PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+ fd = packed_ref_cache->lock->fd;
do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
- 0, write_packed_entry_fn,
- &packed_ref_cache->lock->fd);
+ 0, write_packed_entry_fn, &fd);
if (commit_lock_file(packed_ref_cache->lock)) {
save_errno = errno;
error = -1;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html