Re: [PATCH v2] git-compat-util: work around for access(X_OK) under root
- From
- CHIGOT, CLEMENT <clement.chigot@atos.net>
- Date
- Apr 24, 2019, 13:58 UTC
- Message-ID
- <AM6PR02MB4950DF3BBE222BE90EA7E3ABEA3C0@AM6PR02MB4950.eurprd02.prod.outlook.com>
- In-Reply-To
- <xmqqa7gf1q0y.fsf@gitster-ct.c.googlers.com>
Show 15 quoted lines
> From: Junio C Hamano <gitster@pobox.com> > "CHIGOT, CLEMENT" <clement.chigot@atos.net> writes: > > > On AIX, access(X_OK) may success when run under root even if the > > s/success/succeed/; > > Also perhaps s/under/as/. > > > execution isn't possible. This comes from the POSIX specifications > > s/comes from/behaviour is allowed by/; I agree with you that AIX > behaviour is suboptimal and I do not think we want to give an > impression that POSIX encourages such an illogical behaviour. It > merely is permitted.
Yes, that's better
Show 20 quoted lines
> > > "git commit -m content > > fatal: cannot exec '.git/hooks/pre-commit': Permission denied" > > I am not sure what the double-quotes around these two lines are > about. Perhaps drop them, and if you want to clarify which part is > your word and which part is quoted material, have blank lines to > delineate instead, perhap like: > > ... POSIX says: > > ... for a process with appropriate ... > ... to any user. > > This behaviour can fail the execution of hooks, like so: > > $ git commit > fatal: cannot exec '.git/hooks/pre-commit': Permission denied > > Add NEED_ACCESS_ROOT_HANDLER in order to...
I'm used to do that. But if you think that's clearer, I'll do it.
Show 23 quoted lines
> > > is root. > > > > Signed-off-by: Clément Chigot <clement.chigot@atos.net> > > Message-ID: <AM6PR02MB4950BB0152893633FF95DC99EA230@AM6PR02MB4950.eurprd02.prod.outlook.com> > > Drop "Message-Id:" from the footer. > > > --- > > ... > > diff --git a/compat/access.c b/compat/access.c > > new file mode 100644 > > index 0000000000..fcfaefb0c0 > > --- /dev/null > > +++ b/compat/access.c > > @@ -0,0 +1,30 @@ > > +#define COMPAT_CODE_ACCESS > > I am torn between just using COMPAT_CODE like the other one does, or > introducing the new symbol like this. If we were to do the latter, > perhaps we should give the original one a more specific name as well > (e.g. COMPAT_CODE_FILENO or something like that). >
I think using the same define is possible for now. But it might become confusing if others are needed. Moreover, I'm not sure it will work if, for example, fileno is needed inside git_access. That's very unlikely to happen, but it seems better to have one COMPAT_CODE_X defined for each special handler. I'll let you decide., anyway.
Show 15 quoted lines
> > + if (stat(path, &st) < 0) > > + return -1; > > + > > + /* Root can read or write any file. */ > > + if (!(mode & X_OK)) > > + return 0; > > + > > + /* Root can execute any file that has any one of the execute > > + bits set. */ > > /* > * Our multi-line comment looks like this, > * with opening slash-asterisk and closing > * asterisk-slash on their own lines. > */
.. I've forgotten this one, sorry.
Show 7 quoted lines
> > diff --git a/git-compat-util.h b/git-compat-util.h > > index 31b47932bd..d0cb380522 100644 > > --- a/git-compat-util.h > > +++ b/git-compat-util.h > > @@ -1242,6 +1242,13 @@ int git_fileno(FILE *stream); > > # endif > > #endif
Show 18 quoted lines
> > > > +#ifdef NEED_ACCESS_ROOT_HANDLER > > +int git_access(const char *path, int mode); > > +# ifndef COMPAT_CODE_ACCESS > > Notice that the fileno thing we are trying to mimick protects us > from a system header that defines the macro we are about to define, > which is a good practice to prevent compilers from complaining > against redefinition. We should imitate it like this here: > > # ifdef access > # undef access > # endif > > > +# define access(path, mode) git_access(path, mode) > > +# endif > > +#endif > > +
Ok.
> > Other than that, looks good to me.