This is the mail archive of the cygwin-developers@cygwin.com 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: new blood


Christopher Faylor wrote:

> A case in point is the SYSTEMROOT problem which was recently unearthed
> in the cygwin list.  Corinna fixed it a while ago and then I broke it
> again.  It would have been nice to have a test suite entry which detected
> that.

So, I got to looking into this.  It turns out that this is still broken, but only
in the case where the process being spawned is mounted cygexec.

In build_env() there is the following code to find any missing environment
variables that have been marked as add_always:

  /* Fill in any required-but-missing environment variables. */
  for (unsigned i = 0; i < SPENVS_SIZE; i++)
    if (!saw_spenv[i] && (spenvs[i].add_always || cygheap->user.issetuid ()))
      {
	  *dstp = spenvs[i].retrieve (no_envblock);
	  if (*dstp && (!no_envblock || spenvs[i].force) && *dstp != env_dontadd)
	    {
	      *pass_dstp++ = *dstp;
	      tl += strlen (*dstp) + 1;
	      dstp++;
	    }
	}

This results in a call to spenv::retrieve() for SYSTEMROOT, and since it has a
from_cygheap function defined:

      /* Calculate (potentially) value for given environment variable.  */
      p = (cygheap->user.*from_cygheap) (name, namelen);
      if (!p || (no_envblock && !env) || (p == env_dontadd))
	return env_dontadd;

It correctly retrieves the value from the heap but then decides not to include it
in the environment because no_envblock is true (cygexec) and env is NULL (because
it doesn't exist in the current environment.)  The result is a child process with
no SYSTEMROOT.

I would offer a patch to fix this but I'm not entirely sure of the logic here.  It
seems that changing the above conditional to (!add_always && no_envblock && !env)
might do it.

So, about that testcase...

The 'cygexec' issue aside, I found it very hard to actually make a testcase that
failed on 1.5.16 and passes on current CVS.  I think the reason for this was that
the env.variable was being added to the Cygwin heap, but not being added to the
win32 environment passed to CreateProcess().  The strace shows this difference of
one:

   30   19933 [main] environ 1808 build_env: env count 61, bytes 2890
  140   20073 [main] environ 1808 build_env: envp 0x61811608, envc 62

Writing a test case that would have found this bug is tough, because a Cygwin child
process will see the variable, but a native child process looking at the win32
environment would not find it -- and thus the socket calls would fail, too.

I've got something that does the job, but it's really ugly.  It requires a tiny
mingw helper app.  I'm not sure how that would work with the testcase framework.  I
could have it emit the source to /tmp/whatever.c, call gcc, use the binary, and
then delete it when done.  But that seems hackish.

The alternative I suppose would be to spawn something that tries to do some kind of
socket function... ping 127.0.0.1 maybe?

Are there any other environment-related things that should also be tested?

Brian


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