{"thread":{"id":"13167","subject":"[PATCH 2/8] Add a lockfile function to append to a file","startedAt":"2008-04-17T23:32:26Z","lastAt":"2008-04-17T23:51:39Z","messageCount":3,"participants":["Daniel Barkalow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"74699","messageId":"alpine.LNX.1.00.0804171931480.19665@iabervon.org","threadId":"13167","inReplyTo":null,"subject":"[PATCH 2/8] Add a lockfile function to append to a file","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-04-17T23:32:26Z","receivedAt":"2008-04-17T23:32:26Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This takes care of copying the original contents into the replacement\nfile after the lock is held, so that concurrent additions can't miss\neach other's changes.\n\nSigned-off-by: Daniel Barkalow \n---\n cache.h | 1 +\n lockfile.c | 17 +++++++++++++++++\n 2 files changed, 18 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 2a1e7ec..4a113b0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -391,6 +391,7 @@ struct lock_file {\n \tchar filename[PATH_MAX];\n };\n extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);\n+extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);\n extern int commit_lock_file(struct lock_file *);\n \n extern int hold_locked_index(struct lock_file *, int);\ndiff --git a/lockfile.c b/lockfile.c\nindex 663f18f..95fd3d1 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -160,6 +160,23 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on\n \treturn fd;\n }\n \n+int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)\n+{\n+\tint fd = lock_file(lk, path);\n+\tstruct stat st;\n+\tif (!stat(path, &st)) {\n+\t\tint orig_fd = open(path, O_RDONLY);\n+\t\tsize_t mmap_size = xsize_t(st.st_size);\n+\t\tvoid *mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE,\n+\t\t\t\t orig_fd, 0);\n+\t\twrite_or_die(fd, mmap, mmap_size);\n+\t\tmunmap(mmap, mmap_size);\n+\t}\n+\tif (fd < 0 && die_on_error)\n+\t\tdie(\"unable to create '%s.lock': %s\", path, strerror(errno));\n+\treturn fd;\n+}\n+\n int close_lock_file(struct lock_file *lk)\n {\n \tint fd = lk->fd;\n-- \n1.5.4.3.610.gea6cd\n"},{"id":"74700","messageId":"7v3apkm4lz.fsf@gitster.siamese.dyndns.org","threadId":"13167","inReplyTo":"alpine.LNX.1.00.0804171931480.19665@iabervon.org","subject":"Re: [PATCH 2/8] Add a lockfile function to append to a file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-17T23:41:28Z","receivedAt":"2008-04-17T23:41:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> This takes care of copying the original contents into the replacement\n> file after the lock is held, so that concurrent additions can't miss\n> each other's changes.\n>\n> Signed-off-by: Daniel Barkalow \n> ---\n> cache.h | 1 +\n> lockfile.c | 17 +++++++++++++++++\n> 2 files changed, 18 insertions(+), 0 deletions(-)\n>\n> diff --git a/cache.h b/cache.h\n> index 2a1e7ec..4a113b0 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -391,6 +391,7 @@ struct lock_file {\n> \tchar filename[PATH_MAX];\n> };\n> extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);\n> +extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);\n> extern int commit_lock_file(struct lock_file *);\n> \n> extern int hold_locked_index(struct lock_file *, int);\n> diff --git a/lockfile.c b/lockfile.c\n> index 663f18f..95fd3d1 100644\n> --- a/lockfile.c\n> +++ b/lockfile.c\n> @@ -160,6 +160,23 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on\n> \treturn fd;\n> }\n> \n> +int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)\n> +{\n> +\tint fd = lock_file(lk, path);\n> +\tstruct stat st;\n> +\tif (!stat(path, &st)) {\n> +\t\tint orig_fd = open(path, O_RDONLY);\n> +\t\tsize_t mmap_size = xsize_t(st.st_size);\n> +\t\tvoid *mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE,\n> +\t\t\t\t orig_fd, 0);\n> +\t\twrite_or_die(fd, mmap, mmap_size);\n> +\t\tmunmap(mmap, mmap_size);\n> +\t}\n> +\tif (fd < 0 && die_on_error)\n> +\t\tdie(\"unable to create '%s.lock': %s\", path, strerror(errno));\n\nCopying and checking are the other way around, aren't they? Grab the\nlock, error out if you cannot, then read and copy.\n\n> +\treturn fd;\n> +}\n> +\n> int close_lock_file(struct lock_file *lk)\n> {\n> \tint fd = lk->fd;\n> -- \n> 1.5.4.3.610.gea6cd\n"},{"id":"74701","messageId":"7vy77ckpkk.fsf@gitster.siamese.dyndns.org","threadId":"13167","inReplyTo":"alpine.LNX.1.00.0804171931480.19665@iabervon.org","subject":"Re: [PATCH 2/8] Add a lockfile function to append to a file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-17T23:51:39Z","receivedAt":"2008-04-17T23:51:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow writes:\n\n> +int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)\n> +{\n> +\tint fd = lock_file(lk, path);\n> +\tstruct stat st;\n> +\tif (!stat(path, &st)) {\n> +\t\tint orig_fd = open(path, O_RDONLY);\n> +\t\tsize_t mmap_size = xsize_t(st.st_size);\n> +\t\tvoid *mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE,\n> +\t\t\t\t orig_fd, 0);\n> +\t\twrite_or_die(fd, mmap, mmap_size);\n> +\t\tmunmap(mmap, mmap_size);\n> +\t}\n> +\tif (fd < 0 && die_on_error)\n> +\t\tdie(\"unable to create '%s.lock': %s\", path, strerror(errno));\n> +\treturn fd;\n> +}\n\nAnother glitch. What should we do when stat(path) fails but the file\ncannot be read?\n\nI think the sequence actually should be:\n\n\tfd = lock_file();\n if (fd < 0)\n \terror out;\n\torig_fd = open(path, O_RDONLY);\n if (orig_fd < 0) {\n\t\tif (errno != ENOENT)\n\t\t\tdie(\"unable to open %s\", path);\n\t\tcopy;\n\t}\n\treturn fd;\n"}]}