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 Jan 10 11:56, Erik Bray wrote:
> 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).

It's not that tricky.  In vim terms, set ts=8 sw=2, and use tabs
if the indentation is >= 8.  If you do a >> << sequence in vim,
you get it right even if it was wrong before.

> Would you welcome a separate patch with general whitespace cleanup?

It's new code so that doesn't make much sense.  Otherwise cleaning
up existing stuff as separate patch is fine.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature


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