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

[Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>]

On Wed, Aug 20, 2008 at 10:45:37PM -0700, Jared Hulbert wrote:
>+static int axfs_get_onmedia_super(struct super_block *sb)
>+{
>+	int err;
>+	struct axfs_super *sbi = AXFS_SB(sb);
>+	struct axfs_super_onmedia *sbo;
>+
>+	sbo = kmalloc(sizeof(*sbo), GFP_KERNEL);
>+	if (!sbo)
>+		return -ENOMEM;
>+
>+	axfs_copy_metadata(sb, (void *)sbo, 0, sizeof(*sbo));
>+
>+	/* Do sanity checks on the superblock */
>+	if (be32_to_cpu(sbo->magic) != AXFS_MAGIC) {
>+		printk(KERN_ERR "axfs: wrong magic\n");
>+		err = -EINVAL;
>+		goto out;
>+	}
>+
>+	/* verify the signiture is correct */
>+	if (strncmp(sbo->signature, AXFS_SIGNATURE, sizeof(AXFS_SIGNATURE))) {
>+		printk(KERN_ERR "axfs: wrong axfs signature,"
>+		       " read %s, expected %s\n",
>+		       sbo->signature, AXFS_SIGNATURE);
>+		err = -EINVAL;
>+		goto out;
>+	}

As Phillip mentioned for some other cases, just initialize err to
EINVAL.
>+
>+	sbi->magic = be32_to_cpu(sbo->magic);
>+	sbi->version_major = sbo->version_major;
>+	sbi->version_minor = sbo->version_minor;
>+	sbi->version_sub = sbo->version_sub;
>+	sbi->files = be64_to_cpu(sbo->files);
>+	sbi->size = be64_to_cpu(sbo->size);
>+	sbi->blocks = be64_to_cpu(sbo->blocks);
>+	sbi->mmap_size = be64_to_cpu(sbo->mmap_size);
>+	sbi->cblock_size = be32_to_cpu(sbo->cblock_size);
>+	sbi->timestamp.tv_sec = be64_to_cpu(sbo->timestamp);
>+	sbi->timestamp.tv_nsec = 0;
>+	sbi->compression_type = sbo->compression_type;
>+
>+	err = axfs_set_compression_type(sbi);
>+	if (err)
>+		goto out;
>+
>+	/* If no block or MTD device, adjust mmapable to cover all image */
>+	if (AXFS_NODEV(sb))
>+		sbi->mmap_size = sbi->size;
>+
>+	err = axfs_fill_region_descriptors(sb, sbo);
[as already mentioned the clipped snippet here is unneeded]
>+out:
>+	kfree(sbo);
>+	return err;
>+}

>+int axfs_verify_device_sizes(struct super_block *sb)
>+{
>+	struct axfs_super *sbi = AXFS_SB(sb);
>+	struct mtd_info *mtd0 = AXFS_MTD(sb);
>+	struct mtd_info *mtd1 = AXFS_MTD1(sb);
>+	int sndsize = sbi->size - sbi->mmap_size;
>+
>+	/* Whole FS on one device */
>+	if (mtd0 && !mtd1 && (mtd0->size < sbi->size)) {
>+		printk(KERN_ERR "axfs: ERROR: Filesystem extends beyond end of"
>+		       "MTD! Filesystem cannot be safely mounted!\n");

missing space in "end ofMTD"
You're mixing the style of where you put such a space, so potential
errors are not easy to spot (manually).
e.g.:

>+			printk(KERN_ERR "axfs: ERROR: Mmap segment extends"
>+			       " beyond end of MTD!");
>+			printk(KERN_ERR "mtd name: %s, mtd size: 0x%x, mmap "
>+			       "size: 0x%llx",
>+			       mtd0->name, mtd0->size, sbi->mmap_size);

>+static int axfs_check_options(char *options, struct axfs_super *sbi)
>+{
>+	unsigned long address = 0;
>+	char *iomem = NULL;
>+	unsigned long length = 0;
>+	char *p;
>+	int err = -EINVAL;
>+	substring_t args[MAX_OPT_ARGS];
>+
>+	if (!options)
>+		return 0;
>+
>+	if (!*options)
>+		return 0;
>+
>+	while ((p = strsep(&options, ",")) != NULL) {
>+		int token;
>+		if (!*p)
>+			continue;
>+
>+		token = match_token(p, tokens, args);
>+		switch (token) {
>+		case OPTION_SECOND_DEV:
>+			sbi->second_dev = match_strdup(&args[0]);
>+			if (!(sbi->second_dev)) {
>+				err = -ENOMEM;
>+				goto out;
>+			}
>+			if (!*(sbi->second_dev))
>+				goto bad_value;
>+			break;
>+		case OPTION_IOMEM:
>+			iomem = match_strdup(&args[0]);
>+			if (!(iomem)) {
>+				err = -ENOMEM;
>+				goto out;
>+			}
>+			if (!*iomem)
>+				goto bad_value;
>+			break;
>+		case OPTION_PHYSICAL_ADDRESS_LOWER_X:
>+		case OPTION_PHYSICAL_ADDRESS_UPPER_X:
>+			if (match_hex(&args[0], (int *)&address))
>+				goto out;
>+			if (!address)
>+				goto bad_value;
>+			break;
>+		default:

just:
			goto bad_value;

>+			printk(KERN_ERR
>+			       "axfs: unrecognized mount option \"%s\" "
>+			       "or missing value\n", p);
>+			goto out;
>+		}
>+	}
>+
>+	if (iomem) {
>+		if (address)
>+			goto out;
>+		err = axfs_get_uml_address(iomem, &address, &length);

missing:
		if (err)
			goto out;

>+		kfree(iomem);
>+		sbi->iomem_size = length;
>+		sbi->virt_start_addr = address;
>+	}
>+
>+	sbi->phys_start_addr = address;
>+	return 0;
>+
>+bad_value:
>+	printk(KERN_ERR
>+	       "axfs: unrecognized mount option \"%s\" "
>+	       "or missing value\n", p);
>+out:
>+	if (iomem)
>+		kfree(iomem);

just kfree(iomem);

>+	return err;
>+}
>+

>+static int axfs_statfs(struct dentry *dentry, struct kstatfs *buf)
>+{
>+	struct axfs_super *sbi = AXFS_SB(dentry->d_sb);
>+
>+	buf->f_type = AXFS_MAGIC;
>+	buf->f_bsize = AXFS_PAGE_SIZE;

What will happen if i transfer the filesystem to a box with a different
pagesize?

>+	buf->f_blocks = sbi->blocks;
>+	buf->f_bfree = 0;
>+	buf->f_bavail = 0;
>+	buf->f_files = sbi->files;
>+	buf->f_ffree = 0;
>+	buf->f_namelen = AXFS_MAXPATHLEN;
>+	return 0;
>+}

I think i have seen the string "compessed" in one of your patches,
should be "compressed".