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  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.

>  }
>  
> +
> +char *
> +_pinfo::environ (size_t& n)
> +{
> +  char **env = NULL;
> +  if (!this || !pid)
> +    return NULL;
> +  if (ISSTATE (this, PID_NOTCYGWIN))
> +    {
> +      RTL_USER_PROCESS_PARAMETERS rupp;
> +      HANDLE proc = open_commune_proc_parms (dwProcessId, &rupp);
> +
> +      n = 0;
> +      if (!proc)
> +        return NULL;
> +
> +	  MEMORY_BASIC_INFORMATION mbi;

Whoops, broken indentation again.

> +      SIZE_T envsize;
> +      PWCHAR envblock;
> +	  if (!VirtualQueryEx (proc, rupp.Environment, &mbi, sizeof(mbi)))

And here

> +        {
> +          NtClose (proc);
> +          return NULL;
> +        }
> +
> +      SIZE_T read;
> +      envsize = mbi.RegionSize - ((char*) rupp.Environment - (char*) mbi.BaseAddress);

Stick to max 80 chars per line, please, i.e.

  +      envsize = mbi.RegionSize
                   - ((char*) rupp.Environment - (char*) mbi.BaseAddress);

It might also be prudent to use another cast here, like, say, ptrdiff_t,
since this is for pointer arithmetic only anyway.

> +      envblock = (PWCHAR) cmalloc_abort (HEAP_COMMUNE, envsize);
> +
> +      if (ReadProcessMemory (proc, rupp.Environment, envblock, envsize, &read))
> +        {
> +          env = win32env_to_cygenv (envblock, false);
> +        }

Your function, your choice, but I'd get rid of the curly braces for
oneliners here.

> +
> +      NtClose (proc);
> +    }
> +  else if (pid != myself->pid)
> +    {
> +      commune_result cr = commune_request (PICOM_ENVIRON);
> +      n = cr.n;
> +      return cr.s;
> +    }
> +  else
> +    {
> +      env = cur_environ ();
> +    }

Same here.

> +
> +  if (env == NULL)
> +      return NULL;

Just as here.


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]