Re: [PATCH 06/10] AXFS: axfs_super.c

[Geert Uytterhoeven]

On Wed, 20 Aug 2008, Jared Hulbert wrote:
> --- /dev/null
> +++ b/fs/axfs/axfs_super.c

> +static int axfs_fill_region_data(struct super_block *sb,
> +				 struct axfs_region_desc *region, int force)
> +{
> +	struct axfs_super *sbi = AXFS_SB(sb);
> +	unsigned long addr;
> +	void *buff = NULL;
> +	void *vaddr;
> +	int err = -ENOMEM;
> +	u64 size = region->size;
> +	u64 fsoffset = region->fsoffset;
> +	u64 end = fsoffset + size;
> +	u64 c_size = region->compressed_size;
> +
> +	if (size == 0)
> +		return 0;
> +
> +	if (AXFS_IS_REGION_INCORE(region))
> +		goto incore;
> +
> +	if (AXFS_IS_REGION_COMPRESSED(region))
> +		goto incore;
> +
> +	if (AXFS_IS_REGION_XIP(sbi, region)) {
> +		if ((end > sbi->mmap_size) && (force))
> +			goto incore;
> +		addr = sbi->virt_start_addr;
> +		addr += (unsigned long)fsoffset;
> +		region->virt_addr = (void *)addr;
> +		return 0;
> +	}
> +
> +	if (force)
> +		goto incore;
> +
> +	region->virt_addr = NULL;
> +	return 0;
> +
> +incore:
> +	region->virt_addr = vmalloc(size);
> +	if (!region->virt_addr)
> +		goto out;
> +	vaddr = region->virt_addr;
> +
> +	if (AXFS_IS_REGION_COMPRESSED(region)) {
> +		buff = vmalloc(c_size);
> +		if (!buff)
> +			goto out;
> +		axfs_copy_metadata(sb, buff, fsoffset, c_size);
> +		err = axfs_uncompress_block(vaddr, size, buff, c_size);
                ^^^
axfs_uncompress_block() doesn't return an error code, so `err' is a bit of a
misnomer.

> +		if (!err)
                    ^^^^
Instead, it returns zero for error, hence this check is correct...

> +			goto out;
> +		vfree(buff);
> +	} else {
> +		axfs_copy_metadata(sb, vaddr, fsoffset, size);
> +	}
> +
> +	return 0;
> +
> +out:
> +	if (buff)
> +		vfree(buff);
> +	if (region->virt_addr)
> +		vfree(region->virt_addr);
> +	return err;
               ^^^
... but it will also return zero here, which is not correct.

Probably it's better to let axfs_uncompress_block() return an (negative) error
code (e.g. -EINVAL or -EIO) in case of failure, which you can just let
propagate.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village  Da Vincilaan 7-D1  B-1935 Zaventem  Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@xxxxxxxxxxx
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 Â RPR Brussels
Fortis  BIC GEBABEBB  IBAN BE41293037680010