[PATCH 2/3] Add a _pinfo.environ() method analogous to _pinfo.cmdline(), and others.

Erik Bray erik.m.bray@gmail.com
Tue Jan 10 10:57:00 GMT 2017


On Mon, Jan 9, 2017 at 3:58 PM, Corinna Vinschen
<corinna-cygwin@cygwin.com> wrote:
> On Jan  5 18:39, Erik M. Bray wrote:
>> diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
>> index 1ce6809..a3e376c 100644
>> --- a/winsup/cygwin/pinfo.cc
>> +++ b/winsup/cygwin/pinfo.cc
>> @@ -653,8 +653,29 @@ commune_process (void *arg)
>>       else if (!WritePipeOverlapped (tothem, path, n, &nr, 1000L))
>>         sigproc_printf ("WritePipeOverlapped fd failed, %E");
>>       break;
>> -      }
>> -    }
>> +       }
>> +     case PICOM_ENVIRON:
>> +       {
>> +     sigproc_printf ("processing PICOM_ENVIRON");
>> +     unsigned n = 0;
>> +    char **env = cur_environ ();
>> +    for (char **e = env; *e; e++)
>> +        n += strlen (*e) + 1;
>> +     if (!WritePipeOverlapped (tothem, &n, sizeof n, &nr, 1000L))
>> +       {
>> +         sigproc_printf ("WritePipeOverlapped sizeof argv failed, %E");
>> +       }
>
>           No curlies here, please, just as in sibling cases.
>
>> +     else
>> +       for (char **e = env; *e; e++)
>> +         if (!WritePipeOverlapped (tothem, *e, strlen (*e) + 1, &nr, 1000L))
>> +           {
>> +             sigproc_printf ("WritePipeOverlapped arg %d failed, %E",
>> +                             e - env);
>> +             break;
>> +           }
>> +     break;
>> +       }
>> +     }
>
> Please have another look into the PICOM_ENVIRON case.  Indentation is
> completely broken in this code snippet, as if it has been moved around
> a bit and then left at the wrong spot.

One note on indentation: I tried to be consistent but it's hard
because in that file and others there's a lot of mixing of tabs and
spaces.  I'm happy to get everything cleaned up, I'm just not sure
what the "intended" convention is wrt tabs vs. spaces (I know you're
using the GNU coding standards otherwise).

Would you welcome a separate patch with general whitespace cleanup?

I acknowledge and can fix every other point.

Thanks,
Erik



More information about the Cygwin-patches mailing list