Re: "" as target of symlink kills translator

[Neal H Walfield]

> I have a change that pleases me aesthetically.  Can you tell me if this
> works?  (The indentation in the patch is all messed up by cut&paste in this
> message.)

This does not work at all.

> 
> Index: dir-lookup.c
> ===================================================================
> RCS file: /cvs/hurd/libdiskfs/dir-lookup.c,v
> retrieving revision 1.46
> diff -u -b -p -r1.46 dir-lookup.c
> --- dir-lookup.c     2001/06/16 20:23:09      1.46
> +++ dir-lookup.c     2001/06/19 22:34:54
> @@ -338,7 +338,12 @@ diskfs_S_dir_lookup (struct protid *dirc
>                          dircred, &amt);
>                            if (error)
>                                goto out;
> +                                assert (amt == np->dn_stat.st_size);

This really should be after line 341 as we do not necessarily set amt.
The following would be much better:

          if (diskfs_read_symlink_hook)
            error = (*diskfs_read_symlink_hook)(np, pathbuf);
          if (!diskfs_read_symlink_hook || error == EINVAL)
            {
              error = diskfs_node_rdwr (np, pathbuf,
                                        0, np->dn_stat.st_size, 0,
                                        dircred, &amt);
              assert (error || amt == np->dn_stat.st_size);
            }

>  
> +       if (np->dn_stat.st_size == 0) /* symlink to "" */
> +           path = nextname ?: "";

First, this cannot be "" as we do a lookup on path and looking up ""
fails.  We need to look up ".".  However, we really should be sure that
"." is a directory, therefore, we should look up "./".

Secondly, at line 96, we do:

      /* Find the name of the next pathname component */
      nextname = index (path, '/');

      if (nextname)
        {
          *nextname++ = '\0';
          while (*nextname == '/')
            nextname++;

As you can see, we end up writing into readonly memory.  Additionally

> +             else
> +                 {
>                     if (nextname)
>                         {
>                               pathbuf[np->dn_stat.st_size] = '/';
> @@ -355,8 +360,9 @@ diskfs_S_dir_lookup (struct protid *dirc
>          strcpy (retryname, pathbuf);
>              goto out;
>                  }
> -
>         path = pathbuf;
> +           }
> +
>         if (lastcomp)
>             {
>                   lastcomp = 0;
> @@ -364,8 +370,14 @@ diskfs_S_dir_lookup (struct protid *dirc
>          creation, so clear the flag here. */
>                create = 0;
>                    }
> +
>         diskfs_nput (np);
>           np = 0;
> +
> +       /* A symlink to "" just loops back to the containing directory,
> +            so that is a valid value for PATH right now.  Short-circuit
> +                 the end-of-loop test.  */
> +                   continue;

We cannot call continue if path is ""; I also thought this initially,
however, we still do the end of loop test.

pgpszwnmY5K5h.pgp
Description: PGP signature