libchromeos: Support setting uid/gid of child processes in process.h

libchromeos: Support setting uid/gid of child processes in process.h

Change-Id: Idaf6e635114ec55a8c25cb71cdfaadc45e334d26

R=sosa@chromium.org
BUG=none
TEST=unittests and emerging a dependent executable

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=785f003

[sosa, 2011-04-17 05:12:46]

Cpl quick questions / suggestions.  Neither are blocking though so LGTM

http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc
File chromeos/process.cc (right):

http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc#newcode34
chromeos/process.cc:34: ProcessImpl::ProcessImpl() : pid_(0), uid_(-1), gid_(-1)
{
seems like the default pid should also be -1 since 0 is generally reserved for
the scheduler, but that's not really in your change.

http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc#newcode169
chromeos/process.cc:169: _exit(127);
Was looking at docs for exit codes, is 127 appropriate for all of these error
conditions?

[kmixter1, 2011-04-17 06:54:30]

http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc
File chromeos/process.cc (right):

http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc#newcode34
chromeos/process.cc:34: ProcessImpl::ProcessImpl() : pid_(0), uid_(-1), gid_(-1)
{
On 2011/04/17 05:12:46, sosa wrote:
> seems like the default pid should also be -1 since 0 is generally reserved for
> the scheduler, but that's not really in your change.

I didn't realize that.  However, I guess they are both invalid for pids used by
regular processes.  Seems like -1 would be more correct, though it's not so easy
a fix since vpn-manager looks for == 0.

http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc#newcode169
chromeos/process.cc:169: _exit(127);
On 2011/04/17 05:12:46, sosa wrote:
> Was looking at docs for exit codes, is 127 appropriate for all of these error
> conditions?

Not sure if you mean the man page for exit statuses, or docs for Process itself.
 I added comments about the meaning and an identifier to use instead of 127.  I
believe 127 is the largest exit status and since 0 and 1 are most common, I
wanted to pick one that seems least likely to be used by an actual process.

[sosa, 2011-04-17 20:35:26]

SGTM, LGTM

On Sat, Apr 16, 2011 at 11:54 PM,  <kmixter@chromium.org> wrote:
>
> http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc
> File chromeos/process.cc (right):
>
> http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc#newcode34
> chromeos/process.cc:34: ProcessImpl::ProcessImpl() : pid_(0), uid_(-1),
> gid_(-1) {
> On 2011/04/17 05:12:46, sosa wrote:
>>
>> seems like the default pid should also be -1 since 0 is generally
>
> reserved for
>>
>> the scheduler, but that's not really in your change.
>
> I didn't realize that.  However, I guess they are both invalid for pids
> used by regular processes.  Seems like -1 would be more correct, though
> it's not so easy a fix since vpn-manager looks for == 0.
>
> http://codereview.chromium.org/6865041/diff/1/chromeos/process.cc#newcode169
> chromeos/process.cc:169: _exit(127);
> On 2011/04/17 05:12:46, sosa wrote:
>>
>> Was looking at docs for exit codes, is 127 appropriate for all of
>
> these error
>>
>> conditions?
>
> Not sure if you mean the man page for exit statuses, or docs for Process
> itself.  I added comments about the meaning and an identifier to use
> instead of 127.  I believe 127 is the largest exit status and since 0
> and 1 are most common, I wanted to pick one that seems least likely to
> be used by an actual process.
>
> http://codereview.chromium.org/6865041/
>