All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink)
Date: Sun, 20 Jul 2014 19:13:18 +0700	[thread overview]
Message-ID: <1405858399-23082-1-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1405688937-22925-1-git-send-email-pclouds@gmail.com>

Something extra is, because struct lock_file is usually used as static
variables in many places. This patch reduces bss section by about 80k
bytes (or 23%) on Linux.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 This helps remove the length check in v1 of the next patch.

 cache.h    |  2 +-
 lockfile.c | 56 ++++++++++++++++++++++++++++++++------------------------
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/cache.h b/cache.h
index 44aa439..9ecb636 100644
--- a/cache.h
+++ b/cache.h
@@ -554,7 +554,7 @@ struct lock_file {
 	int fd;
 	pid_t owner;
 	char on_list;
-	char filename[PATH_MAX];
+	char *filename;
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..968b28f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -7,13 +7,19 @@
 static struct lock_file *lock_file_list;
 static const char *alternate_index_output;
 
+static void clear_filename(struct lock_file *lk)
+{
+	free(lk->filename);
+	lk->filename = NULL;
+}
+
 static void remove_lock_file(void)
 {
 	pid_t me = getpid();
 
 	while (lock_file_list) {
 		if (lock_file_list->owner == me &&
-		    lock_file_list->filename[0]) {
+		    lock_file_list->filename) {
 			if (lock_file_list->fd >= 0)
 				close(lock_file_list->fd);
 			unlink_or_warn(lock_file_list->filename);
@@ -77,10 +83,16 @@ static char *last_path_elm(char *p)
  * Always returns p.
  */
 
-static char *resolve_symlink(char *p, size_t s)
+static char *resolve_symlink(const char *in)
 {
+	static char p[PATH_MAX];
+	size_t s = sizeof(p);
 	int depth = MAXDEPTH;
 
+	if (strlen(in) >= sizeof(p))
+		return NULL;
+	strcpy(p, in);
+
 	while (depth--) {
 		char link[PATH_MAX];
 		int link_len = readlink(p, link, sizeof(link));
@@ -124,17 +136,12 @@ static char *resolve_symlink(char *p, size_t s)
 
 static int lock_file(struct lock_file *lk, const char *path, int flags)
 {
-	/*
-	 * subtract 5 from size to make sure there's room for adding
-	 * ".lock" for the lock file name
-	 */
-	static const size_t max_path_len = sizeof(lk->filename) - 5;
-
-	if (strlen(path) >= max_path_len)
+	int len;
+	if (!(flags & LOCK_NODEREF) && !(path = resolve_symlink(path)))
 		return -1;
+	len = strlen(path) + 5; /* .lock */
+	lk->filename = xmallocz(len);
 	strcpy(lk->filename, path);
-	if (!(flags & LOCK_NODEREF))
-		resolve_symlink(lk->filename, max_path_len);
 	strcat(lk->filename, ".lock");
 	lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
 	if (0 <= lk->fd) {
@@ -153,7 +160,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 				     lk->filename);
 	}
 	else
-		lk->filename[0] = 0;
+		clear_filename(lk);
 	return lk->fd;
 }
 
@@ -231,16 +238,17 @@ int close_lock_file(struct lock_file *lk)
 
 int commit_lock_file(struct lock_file *lk)
 {
-	char result_file[PATH_MAX];
-	size_t i;
-	if (lk->fd >= 0 && close_lock_file(lk))
+	char *result_file;
+	if ((lk->fd >= 0 && close_lock_file(lk)) || !lk->filename)
 		return -1;
-	strcpy(result_file, lk->filename);
-	i = strlen(result_file) - 5; /* .lock */
-	result_file[i] = 0;
-	if (rename(lk->filename, result_file))
+	result_file = xmemdupz(lk->filename,
+			       strlen(lk->filename) - 5 /* .lock */);
+	if (rename(lk->filename, result_file)) {
+		free(result_file);
 		return -1;
-	lk->filename[0] = 0;
+	}
+	free(result_file);
+	clear_filename(lk);
 	return 0;
 }
 
@@ -260,11 +268,11 @@ void set_alternate_index_output(const char *name)
 int commit_locked_index(struct lock_file *lk)
 {
 	if (alternate_index_output) {
-		if (lk->fd >= 0 && close_lock_file(lk))
+		if ((lk->fd >= 0 && close_lock_file(lk)) || !lk->filename)
 			return -1;
 		if (rename(lk->filename, alternate_index_output))
 			return -1;
-		lk->filename[0] = 0;
+		clear_filename(lk);
 		return 0;
 	}
 	else
@@ -273,10 +281,10 @@ int commit_locked_index(struct lock_file *lk)
 
 void rollback_lock_file(struct lock_file *lk)
 {
-	if (lk->filename[0]) {
+	if (lk->filename) {
 		if (lk->fd >= 0)
 			close(lk->fd);
 		unlink_or_warn(lk->filename);
 	}
-	lk->filename[0] = 0;
+	clear_filename(lk);
 }
-- 
1.9.1.346.ga2b5940

  parent reply	other threads:[~2014-07-20 12:15 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-18 13:08 [PATCH] Make locked paths absolute when current directory is changed Nguyễn Thái Ngọc Duy
2014-07-18 17:47 ` Junio C Hamano
2014-07-19 12:40   ` Duy Nguyen
2014-07-18 20:44 ` Johannes Sixt
2014-07-20 12:13 ` Nguyễn Thái Ngọc Duy [this message]
2014-07-20 12:13   ` [PATCH v2 2/2] " Nguyễn Thái Ngọc Duy
2014-07-21 13:27     ` Ramsay Jones
2014-07-21 13:47       ` Duy Nguyen
2014-07-21 14:23         ` Ramsay Jones
2014-07-21 17:04         ` Junio C Hamano
2014-07-23 11:55           ` Duy Nguyen
2014-07-31  3:01             ` Yue Lin Ho
2014-07-31  9:58               ` Duy Nguyen
2014-07-20 12:47   ` [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Philip Oakley
2014-07-20 12:50     ` Duy Nguyen
2014-07-31 13:43   ` [PATCH v3 0/3] Keep .lock file paths absolute Nguyễn Thái Ngọc Duy
2014-07-31 13:43     ` [PATCH v3 1/3] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Nguyễn Thái Ngọc Duy
2014-08-01 16:53       ` Junio C Hamano
2014-08-01 17:55         ` Junio C Hamano
2014-08-02 18:13           ` Torsten Bögershausen
2014-08-04 10:13             ` Duy Nguyen
2014-08-04 17:42               ` Junio C Hamano
2014-08-05 16:10               ` Michael Haggerty
2014-09-03  8:00                 ` Yue Lin Ho
2014-08-01 17:34       ` Junio C Hamano
2014-07-31 13:43     ` [PATCH v3 2/3] lockfile.c: remove PATH_MAX limit in resolve_symlink() Nguyễn Thái Ngọc Duy
2014-07-31 13:43     ` [PATCH v3 3/3] lockfile.c: store absolute path Nguyễn Thái Ngọc Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405858399-23082-1-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.