@matheusbsbs / matheus-bagliano / commits / 3096b2ecdb

check_and_freshen_file: fix reversed success-check

When we want to write out a loose object file, we have always first made sure we don't already have the object somewhere. Since 33d4221 (write_sha1_file: freshen existing objects, 2014-10-15), we also update the timestamp on the file, so that a simultaneous prune knows somebody is likely to reference it soon. If our utime() call fails, we treat this the same as not having the object in the first place; the safe thing to do is write out another copy. However, the loose-object check accidentally inverts the utime() check; it returns failure _only_ when the utime() call actually succeeded. Thus it was failing to protect us there, and in the normal case where utime() succeeds, it caused us to pointlessly write out and link the object. This passed our freshening tests, because writing out the new object is certainly _one_ way of updating its utime. So the normal case was inefficient, but not wrong. While we're here, let's also drop a comment in front of the check_and_freshen functions, making a note of their return type (since it is not our usual "0 for success, -1 for error"). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jul 8, 2015 at 16:33 UTC 3096b2ecdb91c27cb9e64b692c480deed6c7e77e

1 file changed +9 -1

sha1_file.c

+9 -1

@@ -442,6 +442,7 @@ void prepare_alt_odb(void)

442 read_info_alternates(get_object_directory(), 0);

443 }

444

445 +/* Returns 1 if we have successfully freshened the file, 0 otherwise. */

446 static int freshen_file(const char *fn)

447 {

448 struct utimbuf t;

@@ -449,11 +450,18 @@ static int freshen_file(const char *fn)

450 return !utime(fn, &t);

451 }

452

453 +/*

454 + * All of the check_and_freshen functions return 1 if the file exists and was

455 + * freshened (if freshening was requested), 0 otherwise. If they return

456 + * 0, you should not assume that it is safe to skip a write of the object (it

457 + * either does not exist on disk, or has a stale mtime and may be subject to

458 + * pruning).

459 + */

460 static int check_and_freshen_file(const char *fn, int freshen)

461 {

462 if (access(fn, F_OK))

463 return 0;

456 - if (freshen && freshen_file(fn))

464 + if (freshen && !freshen_file(fn))

465 return 0;

466 return 1;

467 }