Re: [PATCH 5/6] git_mkstemps_mode: don't set errno to EINVAL for any error.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 20, 2010, 18:13 UTC
- Message-ID
- <7v4olbpyh3.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1266621718-4879-5-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 19 quoted lines
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> > --- > path.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/path.c b/path.c > index 005b836..2886eb6 100644 > --- a/path.c > +++ b/path.c > @@ -222,7 +222,9 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode) > } > /* We return the null string if we can't find a unique file name. */ > pattern[0] = '\0'; > - errno = EINVAL; > + /* Make sure errno signals an error on failure */ > + if (errno <= 0) > + errno = EINVAL; > return -1; > }
Please explain this change a bit better.
A successful call into system library does not have to clear errno (POSIX even says "no function in ... POSIX ... shall set errno to 0"), and you do not clear errno to zero anywhere in this function either, so it looks as if you are reading an undefined errno and basing your action on that result.
Because TMP_MAX is non-zero, you are always reading from errno left by open() in the loop, so the above paragraph is a misunderstanding. But that needs to be in the log message, no?
I think you are trying to avoid stomping on the errno when we broke out of the loop early, due to getting an error. But errno is always valid at this point in this codepath, and errno.h macros shall expand to integer constant expressions with type int, distinct positive values. So I think you can safely remove the assignment without "if (errno <= 0)". Returning EINVAL from a variant of mkstemp when the error is anything but "The last six characters were not XXXXXX" is wrong.