threads / patch / 13167

patch, 8 partsAdd a lockfile function to append to a file

Subject: [PATCH 2/8] Add a lockfile function to append to a file

## tl;dr

3 messages between Apr 17, 2008 and Apr 17, 2008. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Daniel Barkalow· Apr 17, 2008, 23:32 UTC · lore

This takes care of copying the original contents into the replacement file after the lock is held, so that concurrent additions can't miss each other's changes.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 cache.h    |    1 +
 lockfile.c |   17 +++++++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

Show changes to 2 files +18 −0

cache.h, lockfile.c

diff --git a/cache.h b/cache.h
index 2a1e7ec..4a113b0 100644
--- a/cache.h
+++ b/cache.h
@@ -391,6 +391,7 @@ struct lock_file {
 	char filename[PATH_MAX];
 };
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
+extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
 extern int commit_lock_file(struct lock_file *);
 
 extern int hold_locked_index(struct lock_file *, int);
diff --git a/lockfile.c b/lockfile.c
index 663f18f..95fd3d1 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -160,6 +160,23 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on
 	return fd;
 }
 
+int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)
+{
+	int fd = lock_file(lk, path);
+	struct stat st;
+	if (!stat(path, &st)) {
+		int orig_fd = open(path, O_RDONLY);
+		size_t mmap_size = xsize_t(st.st_size);
+		void *mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE,
+				   orig_fd, 0);
+		write_or_die(fd, mmap, mmap_size);
+		munmap(mmap, mmap_size);
+	}
+	if (fd < 0 && die_on_error)
+		die("unable to create '%s.lock': %s", path, strerror(errno));
+	return fd;
+}
+
 int close_lock_file(struct lock_file *lk)
 {
 	int fd = lk->fd;

-- 
1.5.4.3.610.gea6cd

Junio C Hamano· Apr 17, 2008, 23:41 UTC · re: Daniel Barkalow · lore

Re: [PATCH 2/8] Add a lockfile function to append to a file

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 44 quoted lines

> This takes care of copying the original contents into the replacement
> file after the lock is held, so that concurrent additions can't miss
> each other's changes.
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
>  cache.h    |    1 +
>  lockfile.c |   17 +++++++++++++++++
>  2 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 2a1e7ec..4a113b0 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -391,6 +391,7 @@ struct lock_file {
>  	char filename[PATH_MAX];
>  };
>  extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
> +extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
>  extern int commit_lock_file(struct lock_file *);
>  
>  extern int hold_locked_index(struct lock_file *, int);
> diff --git a/lockfile.c b/lockfile.c
> index 663f18f..95fd3d1 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -160,6 +160,23 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on
>  	return fd;
>  }
>  
> +int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)
> +{
> +	int fd = lock_file(lk, path);
> +	struct stat st;
> +	if (!stat(path, &st)) {
> +		int orig_fd = open(path, O_RDONLY);
> +		size_t mmap_size = xsize_t(st.st_size);
> +		void *mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE,
> +				   orig_fd, 0);
> +		write_or_die(fd, mmap, mmap_size);
> +		munmap(mmap, mmap_size);
> +	}
> +	if (fd < 0 && die_on_error)
> +		die("unable to create '%s.lock': %s", path, strerror(errno));

Copying and checking are the other way around, aren't they? Grab the lock, error out if you cannot, then read and copy.

Show 8 quoted lines

> +	return fd;
> +}
> +
>  int close_lock_file(struct lock_file *lk)
>  {
>  	int fd = lk->fd;
> -- 
> 1.5.4.3.610.gea6cd

Junio C Hamano· Apr 17, 2008, 23:51 UTC · re: Daniel Barkalow · lore

Re: [PATCH 2/8] Add a lockfile function to append to a file

Daniel Barkalow <barkalow@iabervon.org> writes:

Show 16 quoted lines

> +int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)
> +{
> +	int fd = lock_file(lk, path);
> +	struct stat st;
> +	if (!stat(path, &st)) {
> +		int orig_fd = open(path, O_RDONLY);
> +		size_t mmap_size = xsize_t(st.st_size);
> +		void *mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE,
> +				   orig_fd, 0);
> +		write_or_die(fd, mmap, mmap_size);
> +		munmap(mmap, mmap_size);
> +	}
> +	if (fd < 0 && die_on_error)
> +		die("unable to create '%s.lock': %s", path, strerror(errno));
> +	return fd;
> +}

Another glitch. What should we do when stat(path) fails but the file cannot be read?

I think the sequence actually should be:

	fd = lock_file();
        if (fd < 0)
        	error out;
	orig_fd = open(path, O_RDONLY);
        if (orig_fd < 0) {
		if (errno != ENOENT)
			die("unable to open %s", path);
		copy;
	}
	return fd;