On Mon, Jan 9, 2017 at 3:43 PM, Corinna Vinschen <[email protected]> wrote: > Hi Erik, > > On Jan 5 18:39, [email protected] wrote: >> From: "Erik M. Bray" <[email protected]> >> >> Per this discussion started in this thread: >> https://cygwin.com/ml/cygwin/2016-11/msg00205.html >> >> I finally got around to finishing a patch for this feature. It supports both >> Cygwin and >> native Windows processes, more or less following the example of how >> /proc/<pid>/cmdline is >> implemented. >> >> Erik M. Bray (3): >> Move the core environment parsing of environ_init into a new >> win32env_to_cygenv function. >> Add a _pinfo.environ() method analogous to _pinfo.cmdline(), and >> others. >> Add a /proc/<pid>/environ proc file handler, analogous to >> /proc/<pid>/cmdline. >> >> winsup/cygwin/environ.cc | 84 +++++++++++++++++++++--------------- >> winsup/cygwin/environ.h | 2 + >> winsup/cygwin/fhandler_process.cc | 22 ++++++++++ >> winsup/cygwin/pinfo.cc | 89 >> ++++++++++++++++++++++++++++++++++++++- >> winsup/cygwin/pinfo.h | 4 +- >> 5 files changed, 163 insertions(+), 38 deletions(-) > > Patch looks good basically, but I have a few nits: > > - We need your 2-clause BSD license text per the "Before you get started" > section of https://cygwin.com/contrib.html. For the text see > https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/CONTRIBUTORS > > - While this appears to work nicely on other processes, it seems to be > broken on the process itself. Did you try `cat /proc/self/environ'? > I'm getting a "Bad address" error when trying that. > > - A few formatting issues, see my next replies. > > Other than that, thanks for this nice addition!
Incidentally, I don't think I did test `/proc/self/environ`. I'll certainly fix whatever's wrong with that. When I fix that and the issues you pointed out on the other patches, should I just submit a new patch set? When I do so I can also include the BSD license statement. Thanks, Erik
