On 8/24/10, Miod Vallat <miod@online.fr> wrote:
quoted text
>> 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

quoted text
> 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])