Re: [PATCH v18 07/48] lockfile.c: make lock_file return a meaningful errno on failurei
From: Michael Haggerty <hidden>
Date: 2016-06-15 23:01:41
On 06/17/2014 05:53 PM, Ronnie Sahlberg wrote:
quoted hunk ↗ jump to hunk
Making errno when returning from lock_file() meaningful, which should fix * an existing almost-bug in lock_ref_sha1_basic where it assumes errno==ENOENT is meaningful and could waste some work on retries * an existing bug in repack_without_refs where it prints strerror(errno) and picks advice based on errno, despite errno potentially being zero and potentially having been clobbered by that point Signed-off-by: Ronnie Sahlberg <redacted> --- lockfile.c | 17 ++++++++++++----- refs.c | 1 + refs.h | 1 + 3 files changed, 14 insertions(+), 5 deletions(-)diff --git a/lockfile.c b/lockfile.c index 464031b..a921d77 100644 --- a/lockfile.c +++ b/lockfile.c@@ -121,7 +121,7 @@ static char *resolve_symlink(char *p, size_t s) return p; } - +/* Make sure errno contains a meaningful value on error */ static int lock_file(struct lock_file *lk, const char *path, int flags) { /*@@ -130,8 +130,10 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) */ static const size_t max_path_len = sizeof(lk->filename) - 5; - if (strlen(path) >= max_path_len) + if (strlen(path) >= max_path_len) { + errno = ENAMETOOLONG; return -1; + } strcpy(lk->filename, path); if (!(flags & LOCK_NODEREF)) resolve_symlink(lk->filename, max_path_len);@@ -148,9 +150,13 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) lock_file_list = lk; lk->on_list = 1; } - if (adjust_shared_perm(lk->filename)) - return error("cannot fix permission bits on %s", - lk->filename); + if (adjust_shared_perm(lk->filename)) { + int save_errno = errno; + error("cannot fix permission bits on %s", + lk->filename); + errno = save_errno; + return -1; + }
Wouldn't it make sense for error() to save and restore errno instead of scattering the save/restore code around everywhere? I saw the same type of code about three commits later, too.
[...]
Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/