This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]