Re: [PATCH 1/2] run-command: Add checks after execvp fails with EACCES

["Frans Klaver" <fransklaver@gmail.com>]

On Tue, 06 Dec 2011 23:35:53 +0100, Junio C Hamano <gitster@pobox.com>  
wrote:

> Frans Klaver <fransklaver@gmail.com> writes:
>
>> +#ifndef WIN32
>> +static int is_in_group(gid_t gid)
>> ...
>> +static int have_read_execute_permissions(const char *path)
>> +{
>> +	struct stat s;
>> +	trace_printf("checking '%s'\n", path);
>> +
>> +	if (stat(path, &s) < 0) {
>> + ...
>> +	/* check world permissions */
>> +	if ((s.st_mode&(S_IXOTH|S_IROTH)) == (S_IXOTH|S_IROTH))
>> +		return 1;
>
> Hmm, do you need to do this with stat(2)?
>
> Wouldn't access(2) with R_OK|X_OK give you exactly what you want without
> this much trouble?

I just had a good look through the man page of access(2), and I think it  
depends. access works for the real uid, which is what I attempted to  
implement in the above check as well. However, do we actually need to use  
the real uid or do we need the set uid (geteuid(2))? Would it be safe to  
assume we don't setuid?


> I also think that your permission check is incorrectly implemented.
>
>     $ cd /var/tmp && date >j && chmod 044 j && ls -l j
>     ----r--r-- 1 junio junio 29 Dec  6 14:32 j
>     $ cat j
>     cat: j: Permission denied
>     $ su pogo
>     Password:
>     $ cat j
>     Tue Dec  6 14:32:23 PST 2011
> That's a world-readable but unreadable-only-to-me file.

Will fix if we can't use access(2) due to what I mentioned above.



>> +				warn("file '%s' exists and permissions "
>> +				"seem OK.\nIf this is a script, see if you "
>> +				"have sufficient privileges to run the "
>> +				"interpreter", sb.buf);
>
> Does "warn()" do the right thing for multi-line strings like this?

Looking back on it, I think I actually wanted to use warning() from  
usage.c. I'll still have to check if that does the multi-line thing as I  
expect it to.