Mail Archives: djgpp-workers/1999/03/01/04:30:33
On Sun, 28 Feb 1999, Mark E. wrote:
> Below are my latest patches to implement chroot.
Thanks!
> +      __CHROOT_NO_SETENV_FLAG is set at startup to avoid
> +      wasting time setting the same environmental variables
> +      that resulted in chroot being called in the first place. */
> +   if (!(__chroot_flags & __CHROOT_NO_SETENV_FLAG))
> +     setenv(ROOT_ENV, __djgpp_root, 1);
I'm not sure this is worth the hassle: putenv (called by setenv)
already avoids most of the work when the old and the new values are
identical.
> + int
> + fchroot (int fd)
> + {
> +   unsigned int flags = __chroot_flags;
> + 
> +   if (fd < 0)
> +   {
> +     errno = EINVAL;
> +     return -1;
> +   }
I think errno should be set here to EBADF, not EINVAL.
> *** src/libc/compat/unistd/chroot.txh.orig	Sun Feb 28 14:25:02 1999
> --- src/libc/compat/unistd/chroot.txh	Sat Feb 27 18:01:34 1999
Please resend the diffs for .txh files, they are garbled in strange
ways (some + marks are missing, and some line beginnings too).
> + Causes @var{new_root_directory} to become the root directory, or 
> starting point, for path name searches beginning with '/' or '\'. The 
> current working directory is unaffected.
Here's an example of a garbled part.
Please put all file names or parts thereof (like '/') inside @file{}.
> {ROOT} is not set, but @var{SYSROOT} is, then @var{SYSROOT} is 
> used to set the root directory, and @code{chroot} is set to permissive 
> mode, and @var{CHROOT_UNIX} is ignored.
Another example of garbled lines.
Environment variables should use @code{}, not @var{}.  The latter is
for something that stands for another piece of text, like parameters
passed to a function.
Also, I think the permissive as opposed to restricted mode should be
explained a bit more, possibly with an example.  It seems to me that
the current text assumes people are familiar with Unix restrictions on
`chroot'.
> + The global variable @var{__chroot_flags} can be set to include the
Same here: use @code{}, since __chroot_flags is the actual name of the
variable.
> + Zero if successful, else nonzero and @var{errno} set if error.
@code{errno}
> +   if (unix_mode)
> +   {
> +     if (*unix_mode == 'Y' || *unix_mode == 'y')
> +       __chroot_flags = __CHROOT_UNIX_MODE_FLAG;
> +     else
> +       __chroot_flags &= ~__CHROOT_UNIX_MODE_FLAG;
> +   }
> +   else if (sysroot)
> +     __chroot_flags &= ~__CHROOT_UNIX_MODE_FLAG;
I think it's better to assign zero to __chroot_flags instead of ANDing
it with ~__CHROOT_UNIX_MODE_FLAG, because otherwise __chroot_flags
with which Emacs was dumped will leak into the restarted program.
Why did you need to use &=, anyway?  __chroot_flags isn't supposed to
have any useful value before this code runs, right?
Also, it seems to me that this code doesn't set __chroot_flags to
anything if none of the environment variables is defined.  This means
that __djgpp_root[] and __djgpp_root_len retain their previous values
(I'm thinking Emacs again).
> +   if (root)
> +   {
> +     __chroot_flags |= __CHROOT_NO_SETENV_FLAG;
> +     /* What should happen when chroot fails? */
> +     ret_code = chroot(root);
I think if chroot fails, you need to abort the program (actually call
`abort').
Where is the call to __crt0_setup_chroot from startup code?  I don't
see it in the diffs.
- Raw text -