@matheusbsbs / matheus-bagliano / commits / cf6950d3bf

lockfile: change lock_file::filename into a strbuf

For now, we still make sure to allocate at least PATH_MAX characters for the strbuf because resolve_symlink() doesn't know how to expand the space for its return value. (That will be fixed in a moment.) Another alternative would be to just use a strbuf as scratch space in lock_file() but then store a pointer to the naked string in struct lock_file. But lock_file objects are often reused. By reusing the same strbuf, we can avoid having to reallocate the string most times when a lock_file object is reused. Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Oct 1, 2014 at 12:28 UTC cf6950d3bfe1447ac04867b1f5654a2fc9c5db96

8 files changed +47 -52

builtin/commit.c

+6 -6

@@ -341,7 +341,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

341 die(_("unable to create temporary index"));

342

343 old_index_env = getenv(INDEX_ENVIRONMENT);

344 - setenv(INDEX_ENVIRONMENT, index_lock.filename, 1);

344 + setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1);

345

346 if (interactive_add(argc, argv, prefix, patch_interactive) != 0)

347 die(_("interactive add failed"));

@@ -352,7 +352,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

352 unsetenv(INDEX_ENVIRONMENT);

353

354 discard_cache();

355 - read_cache_from(index_lock.filename);

355 + read_cache_from(index_lock.filename.buf);

356 if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {

357 if (reopen_lock_file(&index_lock) < 0)

358 die(_("unable to write index file"));

@@ -362,7 +362,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

362 warning(_("Failed to update main cache tree"));

363

364 commit_style = COMMIT_NORMAL;

365 - return index_lock.filename;

365 + return index_lock.filename.buf;

366 }

367

368 /*

@@ -385,7 +385,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

385 if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK))

386 die(_("unable to write new_index file"));

387 commit_style = COMMIT_NORMAL;

388 - return index_lock.filename;

388 + return index_lock.filename.buf;

389 }

390

391 /*

@@ -472,9 +472,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix

472 die(_("unable to write temporary index file"));

473

474 discard_cache();

475 - read_cache_from(false_lock.filename);

475 + read_cache_from(false_lock.filename.buf);

476

477 - return false_lock.filename;

477 + return false_lock.filename.buf;

478 }

479

480 static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,

builtin/reflog.c

+1 -1

@@ -431,7 +431,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,

431 write_str_in_full(lock->lock_fd, "\n") != 1 ||

432 close_ref(lock) < 0)) {

433 status |= error("Couldn't write %s",

434 - lock->lk->filename);

434 + lock->lk->filename.buf);

435 unlink(newlog_path);

436 } else if (rename(newlog_path, log_file)) {

437 status |= error("cannot rename %s to %s",

cache.h

+1 -1

@@ -580,7 +580,7 @@ struct lock_file {

580 volatile int fd;

581 volatile pid_t owner;

582 char on_list;

583 - char filename[PATH_MAX];

583 + struct strbuf filename;

584 };

585 #define LOCK_DIE_ON_ERROR 1

586 #define LOCK_NODEREF 2

config.c

+7 -7

@@ -2024,9 +2024,9 @@ int git_config_set_multivar_in_file(const char *config_filename,

2024 MAP_PRIVATE, in_fd, 0);

2025 close(in_fd);

2026

2027 - if (chmod(lock->filename, st.st_mode & 07777) < 0) {

2027 + if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {

2028 error("chmod on %s failed: %s",

2029 - lock->filename, strerror(errno));

2029 + lock->filename.buf, strerror(errno));

2030 ret = CONFIG_NO_WRITE;

2031 goto out_free;

2032 }

@@ -2106,7 +2106,7 @@ out_free:

2106 return ret;

2107

2108 write_err_out:

2109 - ret = write_error(lock->filename);

2109 + ret = write_error(lock->filename.buf);

2110 goto out_free;

2111

2112 }

@@ -2207,9 +2207,9 @@ int git_config_rename_section_in_file(const char *config_filename,

2207

2208 fstat(fileno(config_file), &st);

2209

2210 - if (chmod(lock->filename, st.st_mode & 07777) < 0) {

2210 + if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {

2211 ret = error("chmod on %s failed: %s",

2212 - lock->filename, strerror(errno));

2212 + lock->filename.buf, strerror(errno));

2213 goto out;

2214 }

2215

@@ -2230,7 +2230,7 @@ int git_config_rename_section_in_file(const char *config_filename,

2230 }

2231 store.baselen = strlen(new_name);

2232 if (!store_write_section(out_fd, new_name)) {

2233 - ret = write_error(lock->filename);

2233 + ret = write_error(lock->filename.buf);

2234 goto out;

2235 }

2236 /*

@@ -2256,7 +2256,7 @@ int git_config_rename_section_in_file(const char *config_filename,

2256 continue;

2257 length = strlen(output);

2258 if (write_in_full(out_fd, output, length) != length) {

2259 - ret = write_error(lock->filename);

2259 + ret = write_error(lock->filename.buf);

2260 goto out;

2261 }

2262 }

lockfile.c

+24 -29

@@ -47,9 +47,9 @@

47 * failed attempt to lock, or a failed close_lock_file()). In this

48 * state:

49 * - active is unset

50 - * - filename[0] == '\0' (usually, though there are transitory states

51 - * in which this condition doesn't hold). Client code should *not*

52 - * rely on this fact!

50 + * - filename is empty (usually, though there are transitory

51 + * states in which this condition doesn't hold). Client code should

52 + * *not* rely on the filename being empty in this state.

53 * - fd is -1

54 * - the object is left registered in the lock_file_list, and

55 * on_list is set.

@@ -170,13 +170,6 @@ static char *resolve_symlink(char *p, size_t s)

170 /* Make sure errno contains a meaningful value on error */

171 static int lock_file(struct lock_file *lk, const char *path, int flags)

172 {

173 - /*

174 - * subtract LOCK_SUFFIX_LEN from size to make sure there's

175 - * room for adding ".lock" for the lock file name:

176 - */

177 - static const size_t max_path_len = sizeof(lk->filename) -

178 - LOCK_SUFFIX_LEN;

179 -

173 if (!lock_file_list) {

174 /* One-time initialization */

175 sigchain_push_common(remove_lock_file_on_signal);

@@ -191,30 +184,32 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)

184 lk->fd = -1;

185 lk->active = 0;

186 lk->owner = 0;

194 - lk->filename[0] = 0;

187 + strbuf_init(&lk->filename, PATH_MAX);

188 lk->next = lock_file_list;

189 lock_file_list = lk;

190 lk->on_list = 1;

191 + } else if (lk->filename.len) {

192 + /* This shouldn't happen, but better safe than sorry. */

193 + die("BUG: lock_file(\"%s\") called with improperly-reset lock_file object",

194 + path);

195 }

196

200 - if (strlen(path) >= max_path_len) {

201 - errno = ENAMETOOLONG;

202 - return -1;

197 + strbuf_addstr(&lk->filename, path);

198 + if (!(flags & LOCK_NODEREF)) {

199 + resolve_symlink(lk->filename.buf, lk->filename.alloc);

200 + strbuf_setlen(&lk->filename, strlen(lk->filename.buf));

201 }

204 - strcpy(lk->filename, path);

205 - if (!(flags & LOCK_NODEREF))

206 - resolve_symlink(lk->filename, max_path_len);

207 - strcat(lk->filename, LOCK_SUFFIX);

208 - lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);

202 + strbuf_addstr(&lk->filename, LOCK_SUFFIX);

203 + lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);

204 if (lk->fd < 0) {

210 - lk->filename[0] = 0;

205 + strbuf_reset(&lk->filename);

206 return -1;

207 }

208 lk->owner = getpid();

209 lk->active = 1;

215 - if (adjust_shared_perm(lk->filename)) {

210 + if (adjust_shared_perm(lk->filename.buf)) {

211 int save_errno = errno;

217 - error("cannot fix permission bits on %s", lk->filename);

212 + error("cannot fix permission bits on %s", lk->filename.buf);

213 rollback_lock_file(lk);

214 errno = save_errno;

215 return -1;

@@ -313,7 +308,7 @@ int reopen_lock_file(struct lock_file *lk)

308 die(_("BUG: reopen a lockfile that is still open"));

309 if (!lk->active)

310 die(_("BUG: reopen a lockfile that has been committed"));

316 - lk->fd = open(lk->filename, O_WRONLY);

311 + lk->fd = open(lk->filename.buf, O_WRONLY);

312 return lk->fd;

313 }

314

@@ -329,9 +324,9 @@ int commit_lock_file(struct lock_file *lk)

324 return -1;

325

326 /* remove ".lock": */

332 - strbuf_add(&result_file, lk->filename,

333 - strlen(lk->filename) - LOCK_SUFFIX_LEN);

334 - err = rename(lk->filename, result_file.buf);

327 + strbuf_add(&result_file, lk->filename.buf,

328 + lk->filename.len - LOCK_SUFFIX_LEN);

329 + err = rename(lk->filename.buf, result_file.buf);

330 strbuf_reset(&result_file);

331 if (err) {

332 int save_errno = errno;

@@ -341,7 +336,7 @@ int commit_lock_file(struct lock_file *lk)

336 }

337

338 lk->active = 0;

344 - lk->filename[0] = 0;

339 + strbuf_reset(&lk->filename);

340 return 0;

341 }

342

@@ -359,8 +354,8 @@ void rollback_lock_file(struct lock_file *lk)

354 return;

355

356 if (!close_lock_file(lk)) {

362 - unlink_or_warn(lk->filename);

357 + unlink_or_warn(lk->filename.buf);

358 lk->active = 0;

364 - lk->filename[0] = 0;

359 + strbuf_reset(&lk->filename);

360 }

361 }

read-cache.c

+2 -2

@@ -2044,10 +2044,10 @@ static int commit_locked_index(struct lock_file *lk)

2044 if (alternate_index_output) {

2045 if (close_lock_file(lk))

2046 return -1;

2047 - if (rename(lk->filename, alternate_index_output))

2047 + if (rename(lk->filename.buf, alternate_index_output))

2048 return -1;

2049 lk->active = 0;

2050 - lk->filename[0] = 0;

2050 + strbuf_reset(&lk->filename);

2051 return 0;

2052 } else {

2053 return commit_lock_file(lk);

refs.c

+3 -3

@@ -2607,8 +2607,8 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)

2607 * lockfile name, minus ".lock":

2608 */

2609 char *loose_filename = xmemdupz(

2610 - lock->lk->filename,

2611 - strlen(lock->lk->filename) - LOCK_SUFFIX_LEN);

2610 + lock->lk->filename.buf,

2611 + lock->lk->filename.len - LOCK_SUFFIX_LEN);

2612 int err = unlink_or_warn(loose_filename);

2613 free(loose_filename);

2614 if (err && errno != ENOENT)

@@ -2972,7 +2972,7 @@ int write_ref_sha1(struct ref_lock *lock,

2972 write_in_full(lock->lock_fd, &term, 1) != 1 ||

2973 close_ref(lock) < 0) {

2974 int save_errno = errno;

2975 - error("Couldn't write %s", lock->lk->filename);

2975 + error("Couldn't write %s", lock->lk->filename.buf);

2976 unlock_ref(lock);

2977 errno = save_errno;

2978 return -1;

shallow.c

+3 -3

@@ -269,8 +269,8 @@ void setup_alternate_shallow(struct lock_file *shallow_lock,

269 if (write_shallow_commits(&sb, 0, extra)) {

270 if (write_in_full(fd, sb.buf, sb.len) != sb.len)

271 die_errno("failed to write to %s",

272 - shallow_lock->filename);

273 - *alternate_shallow_file = shallow_lock->filename;

272 + shallow_lock->filename.buf);

273 + *alternate_shallow_file = shallow_lock->filename.buf;

274 } else

275 /*

276 * is_repository_shallow() sees empty string as "no

@@ -316,7 +316,7 @@ void prune_shallow(int show_only)

316 if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) {

317 if (write_in_full(fd, sb.buf, sb.len) != sb.len)

318 die_errno("failed to write to %s",

319 - shallow_lock.filename);

319 + shallow_lock.filename.buf);

320 commit_lock_file(&shallow_lock);

321 } else {

322 unlink(git_path("shallow"));