Richard W.M. Jones wrote:
 On Wed, Aug 12, 2009 at 10:14:51AM +0200, Jim Meyering wrote:
> Here's one final (I hope) question without a complete patch.
> Given that I already ensure that stubs.c invokes
> REQUIRE_ROOT_OR_RESOLVE_DEVICE just before calling do_umount,
> does this new version of do_umount look ok?
>
> /* Again, use the external /bin/umount program, so that /etc/mtab
>  * is kept updated.
>  */
> int
> do_umount (const char *pathordevice)
> {
>   int r;
>   char *err;
>
>   char *buf = sysroot_path (pathordevice);
>   if (buf == NULL) {
>     reply_with_perror ("malloc");
>     return -1;
>   }
 I think the problem here is you'll end up with a buf like
 '/sysroot/dev/vda'.
 In the original code:
   if (strncmp (pathordevice, "/dev/", 5) == 0) {
     buf = pathordevice;
     IS_DEVICE (buf, -1);
   } else {
     buf = sysroot_path (pathordevice);
     if (buf == NULL) {
       reply_with_perror ("malloc");
       return -1;
     }
     freeit = 1;
   }
 it avoids prefixing /dev paths. 
No need for ugliness after all.
Here's a more faithful new version:
/* Again, use the external /bin/umount program, so that /etc/mtab
 * is kept updated.
 */
int
do_umount (const char *pathordevice)
{
  int r;
  char *err;
  char *buf = (strncmp (pathordevice, "/dev/", 5) == 0
               ? strdup (pathordevice)
               : sysroot_path (pathordevice));
  if (buf == NULL) {
    reply_with_perror ("malloc");
    return -1;
  }
  r = command (NULL, &err, "umount", buf, NULL);
  free (buf);
  if (r == -1) {
    reply_with_error ("umount: %s: %s", pathordevice, err);
    free (err);
    return -1;
  }
  free (err);
  /* update root_mounted? */
  return 0;
}