On 8/24/10, Miod Vallat <[email protected]> wrote:
>> the little operating system we wrote eventually panic'd (overflowed
>> heap);  standalone ufs.c implementation we've used leaks f_buf
>> everytime ufs_open fails
>
> This is correct, but you may be leaking fp->f_blk[] as well. What about
> this instead?

yup i think it's better

ok and thx

> Index: ufs.c
> ===================================================================
> RCS file: /cvs/src/sys/lib/libsa/ufs.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 ufs.c
> --- ufs.c     6 Jan 2008 11:17:18 -0000       1.19
> +++ ufs.c     23 Aug 2010 22:03:12 -0000
> @@ -98,6 +98,7 @@ static int  read_inode(ino_t, struct open
>  static int   block_map(struct open_file *, daddr_t, daddr_t *);
>  static int   buf_read_file(struct open_file *, char **, size_t *);
>  static int   search_directory(char *, struct open_file *, ino_t *);
> +static int   ufs_close_internal(struct file *);
>  #ifdef COMPAT_UFS
>  static void  ffs_oldfscompat(struct fs *);
>  #endif
> @@ -526,10 +527,9 @@ ufs_open(char *path, struct open_file *f
>  out:
>       if (buf)
>               free(buf, fs->fs_bsize);
> -     if (rc) {
> -             free(fp->f_fs, SBSIZE);
> -             free(fp, sizeof(struct file));
> -     }
> +     if (rc)
> +             (void)ufs_close_internal(fp);
> +
>       return (rc);
>  }
>
> @@ -537,11 +537,18 @@ int
>  ufs_close(struct open_file *f)
>  {
>       struct file *fp = (struct file *)f->f_fsdata;
> -     int level;
>
>       f->f_fsdata = (void *)0;
>       if (fp == (struct file *)0)
>               return (0);
> +
> +     return (ufs_close_internal(fp));
> +}
> +
> +static void
> +ufs_close_internal(struct file *fp)
> +{
> +     int level;
>
>       for (level = 0; level < NIADDR; level++) {
>               if (fp->f_blk[level])

Reply via email to