0

I am working on Assignment 2 of ops-class.

The following function bootstraps a file handler array for the process that is being created (eg. user processes for test programs provided here).

int _fh_bootstrap(struct fharray *fhs){

/* Initialize the file handle array of this process */
fharray_init(fhs);

/* String variables initialized for passage to vfs_open */
char* console_inp = kstrdup(CONSOLE); // CONSOLE = "con:"
char* console_out = kstrdup(console_inp);
char* console_err = kstrdup(console_inp);

/* Return variable */
int ret = 0;

/* Initialize the console files STDIN, STDOUT and STDERR */
struct vnode *stdin;
ret = vfs_open(console_inp,O_RDONLY,0,&stdin);
if(ret != 0){
    return ret;
}
kfree(console_inp);

struct fh *stdinfh = kmalloc(sizeof(struct fh));
ret =  _fh_create(O_RDONLY,stdin,stdinfh);
if(ret != 0){
    return ret;
}

stdinfh->fd = STDIN_FILENO;
fharray_add(fhs,stdinfh,NULL);

struct vnode *stdout;
ret = vfs_open(console_out,O_WRONLY,0,&stdout);
if(ret != 0){
    return ret;
}
kfree(console_out);

struct fh *stdoutfh = kmalloc(sizeof(struct fh));
ret =  _fh_create(O_WRONLY,stdout,stdoutfh);
if(ret != 0){
    return ret;
}

stdoutfh->fd = STDOUT_FILENO;
fharray_add(fhs,stdoutfh,NULL);

struct vnode *stderr;
ret = vfs_open(console_err,O_WRONLY,0,&stderr);
if(ret != 0){
    return ret;
}
kfree(console_err);

struct fh *stderrfh = kmalloc(sizeof(struct fh));
ret =  _fh_create(O_WRONLY,stderr,stderrfh);
if(ret != 0){
    return ret;
}

stderrfh->fd = STDERR_FILENO;
fharray_add(fhs,stderrfh,NULL);

fharray_setsize(fhs,MAX_FD);    

return 0;

/* Initialization of stdin, out and err filehandlers complete */
}

If I use os161-gdb to step through this function, I notice the following:

//*stdinfh after the call to _fh_create 
{fd = 0, flag = 0, fh_seek = 0, fh_vnode = 0x80044ddc}

//**stdinfh->fh_vnode
{vn_refcount = 2, vn_countlock = {splk_lock = 0, splk_holder = 0x0}, vn_fs = 0x0,
vn_data = 0x8004ab60, vn_ops = 0x8003e690 <dev_vnode_ops>}

This is the strange part. After stepping through the second call to kmalloc (to init stdoutfh), the stdinfh->fh_vnode pointer changes value!

//**stdinfh->fh_vnode
(struct vnode *) 0x1

And even stranger, after proceeding to the following line

fharray_add(fhs,stdoutfh,NULL);

The value of *stdoutfh->fh_vnode and *stdinfh->fh_vnode IS THE SAME

1 possible explanation: Does the OS not have enough heap memory. I find it unlikely and even after assuming this, I can't exactly explain what is happening here.

Some extra code

  1. _fh_create
  2. struct fh definition

    static int _fh_create(int flag, struct vnode *file, struct fh *handle){
    
    KASSERT(file != NULL);
    
    /* W , R , RW */
    if ( 
        ((flag & O_RDONLY) && (flag & O_WRONLY)) ||
        ((flag & O_RDWR) && ((flag & O_RDONLY) || (flag & O_WRONLY)))
    ) {
        handle = NULL;
        return 1;
    }
    
    handle->flag = flag;
    handle->fh_seek = 0;
    handle->fh_vnode = &file;
    
    return 0;
    }
    

    struct fh { uint32_t fd; // file descriptor int flag; // File handler mode off_t fh_seek; // seek position in file struct vnode **fh_vnode; // File object of the file }

Definition of struct vnode can be found here.

Please let me know if you need more info and thanks for the help!

Deepak
  • 341
  • 4
  • 15
  • 4
    `handle->fh_vnode = &file;` will be a dangling pointer when the function returns. function parameters are automatic variables (stack) – M.M Jan 10 '17 at 04:10
  • Your quote of gdb for the changed node value gives it the wrong type -- * rather than **. Is that a typo or have you changed the types recently? – Richard Urwin Jan 10 '17 at 10:32
  • @M.M So I am not sure I understand your point completely. Do you mean to say that a new variable(s) is created inside the stack frame of _fh_create and this variable is deleted after the function returns? The values *stdoutfh->fh_vnode and *stdinfh->fh_vnode end up being the same because they point to the same offset of the temporary stack frames (_fh_create)? – Deepak Jan 11 '17 at 02:10
  • @Deepak yes that's pretty much it – M.M Jan 11 '17 at 02:12
  • @M.M hmm, I had thought of this but I don't remember why I didn't make the appropriate changes. How should I change my function to work around this 'side-effect'? – Deepak Jan 11 '17 at 02:13
  • @Deepak redesign your code to not make a pointer to stack space like that... maybe `struct fh` should store `file` instead of `&file` ? – M.M Jan 11 '17 at 02:16
  • @RichardUrwin Yes that is on purpose, but it doesn't matter because stdin->fh_vnode and stdoutfh->fh_vnode were also the same i.e they were pointing to the same location in memory. – Deepak Jan 11 '17 at 02:16
  • @M.M Thank you for the solution! I think it worked. Could you post an answer so that I can accept it? – Deepak Jan 12 '17 at 01:41

1 Answers1

1

The code handle->fh_vnode is setting a pointer to automatic variable ("on the stack"), function parameters are automatic variables. After the function returns this will be a dangling pointer.

To fix this you will need to re-design your code a bit, e.g. perhaps the struct fh should just store file and not a pointer to file.

M.M
  • 138,810
  • 21
  • 208
  • 365