From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: Re: [PATCH 2/8] Add a lockfile function to append to a file Date: Sat, 19 Apr 2008 21:59:55 -0700 Message-ID: <7vtzhxce9g.fsf@gitster.siamese.dyndns.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: git@vger.kernel.org, Johannes Schindelin To: Daniel Barkalow X-From: git-owner@vger.kernel.org Sun Apr 20 07:01:21 2008 connect(): Connection refused Return-path: Envelope-to: gcvg-git-2@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1JnRfy-0000OO-HZ for gcvg-git-2@gmane.org; Sun, 20 Apr 2008 07:01:18 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750892AbYDTFAK (ORCPT ); Sun, 20 Apr 2008 01:00:10 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750924AbYDTFAK (ORCPT ); Sun, 20 Apr 2008 01:00:10 -0400 Received: from a-sasl-fastnet.sasl.smtp.pobox.com ([207.106.133.19]:34916 "EHLO sasl.smtp.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750885AbYDTFAJ (ORCPT ); Sun, 20 Apr 2008 01:00:09 -0400 Received: from localhost.localdomain (localhost [127.0.0.1]) by a-sasl-fastnet.sasl.smtp.pobox.com (Postfix) with ESMTP id 9CE672495; Sun, 20 Apr 2008 01:00:07 -0400 (EDT) Received: from pobox.com (ip68-225-240-77.oc.oc.cox.net [68.225.240.77]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by a-sasl-fastnet.sasl.smtp.pobox.com (Postfix) with ESMTP id C7C642490; Sun, 20 Apr 2008 01:00:02 -0400 (EDT) In-Reply-To: (Daniel Barkalow's message of "Thu, 17 Apr 2008 20:02:31 -0400 (EDT)") User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: Daniel Barkalow writes: > 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 > --- > How about this? Also doesn't leak a fd and catches trying to append to a > file you can't read. Should I worry about mmap failing after the open? We have copy.c to copy small existing files, while detecting failure to copy properly. How about doing something like this instead? cache.h | 1 + lockfile.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 0 deletions(-) diff --git a/cache.h b/cache.h index 50b28fa..8d066bf 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..e9e0095 100644 --- a/lockfile.c +++ b/lockfile.c @@ -160,6 +160,34 @@ 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, orig_fd; + + fd = lock_file(lk, path); + if (fd < 0) { + if (die_on_error) + die("unable to create '%s.lock': %s", path, strerror(errno)); + return fd; + } + + orig_fd = open(path, O_RDONLY); + if (orig_fd < 0) { + if (errno != ENOENT) { + if (die_on_error) + die("cannot open '%s' for copying", path); + close(fd); + return error("cannot open '%s' for copying", path); + } + } else if (copy_fd(orig_fd, fd)) { + if (die_on_error) + exit(128); + close(fd); + return -1; + } + return fd; +} + int close_lock_file(struct lock_file *lk) { int fd = lk->fd;