Re: [PATCH/RFC 4/4] Add interactive mode to git-shell for user-friendliness

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



It looks like git@ got dropped from the CC at some point, but I had
written a few days ago:

>> Is there a reason that you duplicate functionality offered by run_command()?
> No--I hadn't realized that existed.  I'll switch over to that in V2 of
> this patch series.

Today, I inspected run_command() in more detail.  Unfortunately, I'm
not sure of the best way to use it in this situation.  In particular,
run_command() uses execvp, meaning that PATH is invoked.  However, the
user should only be able to run commands in the git-shell-commands
directory.   I do have a few ideas for approaches here; maybe others
see more?  Anyway:
- Set PATH to just $HOME/git-shell-commands.  But then the helper
scripts have to restore PATH to a sane value themselves, and it's not
really clear to me what that value should be.
- Under the hood, exec a different script, which processes the user's
command on its own.  (So if the user types 'help' at the git shell
prompt, actually exec 'git-shell-wrapper help'.)  The
git-shell-wrapper could be a dumb wrapper that just execs
$HOME/git-shell-commands/help or similar.
- Extend run_command to optionally use execv.  Would any other code
actually want this functionality though?  If not, it's probably an
excessively large code change for little benefit.
- Continue using the one-off run() method that I wrote here.

Do people have opinions on the most elegant way to handle this?

Thanks!

Greg

> On Wed, Jul 14, 2010 at 12:07 PM, Bernhard R. Link <brlink@xxxxxxxxxx> wrote:
>> * Thomas Rast <trast@xxxxxxxxxxxxxxx> [100714 17:41]:
>>> [Please don't trim the Cc list without good reason.]
>>
>> The mail I answered to had only git@xxxxxxxxxxxxxxx in CC and some
>> syntax errors in To.
>>
>>> Bernhard R. Link wrote:
>>> > To be more specific: If NULL is (void *)0 then it does not need to be
>>> > cast. Sadly the standard allows to define it as 0, and so it is on
>>> > some systems. So to be portable it needs to be cast to be a pointer,
>>> > otherwise the varargs argument is assumed to be an int.
>>>
>>> Worse, the pointer representations need not be the same between types,
>>> even though that is a fairly exotic idea:
>>>
>>>   http://c-faq.com/null/machexamp.html
>>>
>>> So it seems execl() must always have an explicitly-cast (char*)NULL
>>> sentinel.
>>
>> There is a difference between ugly operating systems where everything
>> else works and you need to cast it and things too exotic to have any
>> chance to get the rest of the code to work without big changes.
>>
>> Machines where you do not get a NULL pointer by a memset(,0,), calloc
>> or the like will have bigger problems anyway. (have not looked at git,
>> but I'd be suprised if at every place there is an explicit assignment
>> for the pointers).
>> Note that in the other examples, char * and void * are the same anyway.
>>
>>        Bernhard R. Link
>>
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]