Thread (65 messages) flat view 65 messages, 3 authors, 2016-06-15

Re: [PATCH v10 22/44] fetch.c: clear errno before calling functions that might set it

From: Michael Haggerty <hidden>
Date: 2016-06-15 23:01:13

On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
In s_update_ref there are two calls that when they fail we return an error
based on the errno value. In particular we want to return a specific error
if ENOTDIR happened. Both these functions do have failure modes where they
may return an error without updating errno, in which case a previous and
unrelated ENOTDIR may cause us to return the wrong error. Clear errno before
calling any functions if we check errno afterwards.
If I understand correctly, this is a workaround for some other broken
functions that don't handle errno correctly.  Long-term, wouldn't it be
better to fix the other functions?  In other words, they should change
errno if an only if they return an error status, no?

Of course you are under no obligation to fix the universe, so this
change may be an expedient workaround anyway.  But if you go this route,
then I think a comment would be helpful to explain the unusual clearing
of errno.

Michael
quoted hunk ↗ jump to hunk
Also skip initializing a static variable to 0. Statics live in .bss and
are all automatically initialized to 0.

Signed-off-by: Ronnie Sahlberg <redacted>
---
 builtin/fetch.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..a93c893 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,7 +44,7 @@ static struct transport *gtransport;
 static struct transport *gsecondary;
 static const char *submodule_prefix = "";
 static const char *recurse_submodules_default;
-static int shown_url = 0;
+static int shown_url;
 
 static int option_parse_recurse_submodules(const struct option *opt,
 				   const char *arg, int unset)
@@ -382,6 +382,8 @@ static int s_update_ref(const char *action,
 	if (!rla)
 		rla = default_rla.buf;
 	snprintf(msg, sizeof(msg), "%s: %s", rla, action);
+
+	errno = 0;
 	lock = lock_any_ref_for_update(ref->name,
 				       check_old ? ref->old_sha1 : NULL,
 				       0, NULL);

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help