a recursive function to manipulate a given path

Question

I am working on modifying the didactic OS xv6 (written in c) to support symbolic links (AKA shortcuts). A symbolic link is a file of type T_SYM that contains a path to it's destination. For doing that, i wrote a recursive function that gets a path and a buffer and fills the buffer with the "real" path (i.e. if the path contains a link, it should be replaced by the real path, and a link can occur at any level in the path).

Basically, if i have a path a/b/c/d, and a link from f to a/b, the following operations should be equivalent:

cd a/b/c/d

cd f/c/d

Now, the code is written, but the problem that i try to solve is the problem of starting the path with "/" (meaning that the path is absolute and not relative). Right now, if i run it with a path named /dir1 it treats it like dir1 (relative instead of absolute).

This is the main function, it calls the recursive function. pathname is the given path, buf will contain the real path.

int readlink(char *pathname, char *buf, size_t bufsize){
    char name[DIRSIZ];
    char realpathname[100];

    memset(realpathname,0,100);
    realpathname[0] = '/';

    if(get_real_path(pathname, name, realpathname, 0, 0)){
       memmove(buf, realpathname, strlen(realpathname));
       return strlen(realpathname);
    }

    return -1; 
}

This is the recursive part. the function returns an inode structure (which represents a file or directory in the system). it builds the real path inside realpath. ilock an iunlock are being used to use the inode safely.

struct inode* get_real_path(char *path, char *name, char* realpath, int position){
    struct inode *ip, *next;
    char buf[100];
    char newpath[100];

    if(*path == '/')
        ip = iget(ROOTDEV, ROOTINO);// ip gets the root directory
    else
        ip = idup(proc->cwd); // ip gets the current working directory

    while((path = skipelem(path, name)) != 0){name will get the next directory in the path, path will get the rest of the directories
        ilock(ip);
        if(ip->type != T_DIR){//if ip is a directory
            realpath[position-1] = '\0';
            iunlockput(ip);
            return 0;
        }

        if((next = dirlookup(ip, name, 0)) == 0){//next will get the inode of the next directory
            realpath[position-1] = '\0';
            iunlockput(ip);
            return 0;
        }
        iunlock(ip);

        ilock(next);

        if (next->type == T_SYM){ //if next is a symbolic link
            readi(next, buf, 0, next->size); //buf contains the path inside the symbolic link (which is a path)
            buf[next->size] = 0;
            iunlockput(next);

            next = get_real_path(buf, name, newpath, 0);//call it recursively (might still be a symbolic link)

            if(next == 0){
                realpath[position-1] = '\0';
                iput(ip);

                return 0;
            }

            name = newpath;
            position = 0;
        }     
        else
        iunlock(next);

        memmove(realpath + position, name, strlen(name));
        position += strlen(name);
        realpath[position++]='/';
        realpath[position] = '\0';

        iput(ip);

        ip = next;
    }  
    realpath[position-1] = '\0';

    return ip;
}

I have tried many ways to do it right but with no success. If anyone sees the problem, i'd be happy to hear the solution. Thanks, Eyal

so what happens with this code? does it crush? or give the wrong answer? or just keeps running saying nothing? — Qnan, Jun 27 '12 at 22:31
Without knowing the error you're facing, it's hard to make concrete suggestions; but I'm skeptical of both (a) a recursive function in the kernel and (b) building complete paths inside the kernel. If you continue to use a recursive function you'll eventually need to limit the depth of the recursion to avoid blowing through all available kernel memory. I'm also surprised you're editing paths on the fly -- walking each individual path component at a time feels more plausible to me than building up a potentially huge new string to describe the "real" path... — sarnold, Jun 27 '12 at 22:39
@MikhailKozhevnikov - the code gives the wrong answer (as i have explained, when entering a path starting with a slash, which is an absolute path, it misbehaves) — Eyal, Jun 27 '12 at 22:47
@sarnold - actually there was a piece of code which limits the depth to 16 links, but i removed it to simplify it as much as i can. The reason why i have decided to edit the path on the fly is that it is part of an assignment and the requirement for this part was to implement the function "readlink" which returns the real path of a link — Eyal, Jun 27 '12 at 22:50
.. Incidentally, the `readlink()` function on other systems is _far_ simpler -- it just reads the specific link out of a symbolic link and returns it to the user. If the named file is not a symbolic link, it fails (`EINVAL`). — sarnold, Jun 27 '12 at 22:51
this way it was much simpler, but in this version, readlink() should dereference any symbolic links encountered in the path. — Eyal, Jun 27 '12 at 22:56
I haven't had much experience with xv6, but... can't you just step through that thing and find out what's wrong? I personally have trouble debugging someone else's code just by looking at it :) — Qnan, Jun 28 '12 at 15:16

score 0 · Answer 1 · answered Jun 28 '12 at 15:47

I think it's clear that after running get_real_path(pathname, name, realpathname, 0, 0) the realpathname cannot possibly start with a slash.

Provided the function executes successfully, the memmove(realpath + position, name, strlen(name)) ensures that realpath starts with name, as the position variable always contains zero at the first invocation of memmove. I'd suggest something like

if(*path == '/') {
   ip = iget(ROOTDEV, ROOTINO); // ip gets the root
   realpath[position++] = '/';  
} else
   ip = idup(proc->cwd); // ip gets the current working directory

P.S. I'm not sure why you put a slash into the realpathname before executing the get_real_path, since at this point you don't really know whether the path provided is an absolute one.

score 0 · Accepted Answer · answered Jul 01 '12 at 08:14

Ok, found the problem... The problem was deeper than what i thought... Somehow the realpath was changed sometimes with no visible reason... but the reason was the line: name = newpath;

the solution was to change that line to strcpy(name,newpath);

the previous line made a binding between the name and the realpath... which can be ok if we were not dealing with softlinks. When dereferencing a subpath, this binding ruined everything.

Thanks for the attempts

a recursive function to manipulate a given path

2 Answers2