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

From: Sven Wegener
Date: Thu Aug 21 2008 - 05:07:33 EST


On Wed, 20 Aug 2008, Jared Hulbert wrote:

> The many different devices AXFS can mount to and the various
> dual device mounting schemes are supported here.
>
> Signed-off-by: Jared Hulbert <jaredeh@xxxxxxxxx>
> ---
> diff --git a/fs/axfs/axfs_super.c b/fs/axfs/axfs_super.c
> new file mode 100644
> index 0000000..5efab38
> --- /dev/null
> +++ b/fs/axfs/axfs_super.c
> @@ -0,0 +1,864 @@
> +/*
> + * Advanced XIP File System for Linux - AXFS
> + * Readonly, compressed, and XIP filesystem for Linux systems big and small
> + *
> + * Copyright(c) 2008 Numonyx
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * Authors:
> + * Eric Anderson
> + * Jared Hulbert <jaredeh@xxxxxxxxx>
> + * Sujaya Srinivasan
> + * Justin Treon
> + *
> + * More info and current contacts at http://axfs.sourceforge.net
> + *
> + * axfs_super.c -
> + * Contains the core code used to mount the fs.
> + *
> + */
> +
> +#include <linux/axfs.h>
> +#include <linux/fs.h>
> +#include <linux/vmalloc.h>
> +#include <linux/parser.h>
> +#include <linux/statfs.h>
> +#include <linux/module.h>
> +#include <linux/mount.h>
> +#include <linux/mtd/mtd.h>
> +
> +/******************** Function Declarations ****************************/
> +static struct super_operations axfs_sops;
> +static struct axfs_super *axfs_get_sbi(void);
> +static void axfs_put_sbi(struct axfs_super *);

If you reorder the code slightly you can avoid these static forward
declarations.

> +/***************** functions in other axfs files ***************************/
> +int axfs_get_sb_bdev(struct file_system_type *, int, const char *,
> + struct axfs_super *, struct vfsmount *, int *);
> +void axfs_kill_block_super(struct super_block *);
> +int axfs_copy_block(struct super_block *, void *, u64, u64);
> +int axfs_is_dev_bdev(char *);
> +int axfs_map_mtd(struct super_block *);
> +void axfs_unmap_mtd(struct super_block *);
> +int axfs_copy_mtd(struct super_block *, void *, u64, u64);
> +int axfs_get_sb_mtd(struct file_system_type *, int, const char *,
> + struct axfs_super *, struct vfsmount *, int *);
> +int axfs_is_dev_mtd(char *, int *);
> +void axfs_kill_mtd_super(struct super_block *);
> +struct inode *axfs_create_vfs_inode(struct super_block *, int);
> +int axfs_get_uml_address(char *, unsigned long *, unsigned long *);
> +int axfs_init_profiling(struct axfs_super *);
> +int axfs_shutdown_profiling(struct axfs_super *);
> +void axfs_profiling_add(struct axfs_super *, unsigned long, unsigned int);
> +struct inode *axfs_create_vfs_inode(struct super_block *, int);
> +/******************************************************************************/

These should probably be in axfs.h.

> +static int axfs_fill_region_descriptors(struct super_block *sb,
> + struct axfs_super_onmedia *sbo)
> +{
> + struct axfs_super *sbi = AXFS_SB(sb);
> + struct axfs_region_desc_onmedia *out;
> +
> + out = kmalloc(sizeof(*out), GFP_KERNEL);
> + if (!out)
> + return -ENOMEM;
> + memset(out, 0, sizeof(*out));

kzalloc()

> +
> + axfs_fill_region_desc(sb, out, sbo->strings, &sbi->strings);
> + axfs_fill_region_desc(sb, out, sbo->xip, &sbi->xip);
> + axfs_fill_region_desc(sb, out, sbo->compressed, &sbi->compressed);
> + axfs_fill_region_desc(sb, out, sbo->byte_aligned, &sbi->byte_aligned);
> + axfs_fill_region_desc(sb, out, sbo->node_type, &sbi->node_type);
> + axfs_fill_region_desc(sb, out, sbo->node_index, &sbi->node_index);
> + axfs_fill_region_desc(sb, out, sbo->cnode_offset, &sbi->cnode_offset);
> + axfs_fill_region_desc(sb, out, sbo->cnode_index, &sbi->cnode_index);
> + axfs_fill_region_desc(sb, out, sbo->banode_offset, &sbi->banode_offset);
> + axfs_fill_region_desc(sb, out, sbo->cblock_offset, &sbi->cblock_offset);
> + axfs_fill_region_desc(sb, out, sbo->inode_file_size,
> + &sbi->inode_file_size);
> + axfs_fill_region_desc(sb, out, sbo->inode_name_offset,
> + &sbi->inode_name_offset);
> + axfs_fill_region_desc(sb, out, sbo->inode_num_entries,
> + &sbi->inode_num_entries);
> + axfs_fill_region_desc(sb, out, sbo->inode_mode_index,
> + &sbi->inode_mode_index);
> + axfs_fill_region_desc(sb, out, sbo->inode_array_index,
> + &sbi->inode_array_index);
> + axfs_fill_region_desc(sb, out, sbo->modes, &sbi->modes);
> + axfs_fill_region_desc(sb, out, sbo->uids, &sbi->uids);
> + axfs_fill_region_desc(sb, out, sbo->gids, &sbi->gids);
> +
> + kfree(out);
> +
> + return 0;
> +}


> +int axfs_set_compression_type(struct axfs_super *sbi)

static

And this function actually doesn't set anything, check might be a better
name.

> +{
> + if (sbi->compression_type != ZLIB)
> + return -EINVAL;
> +
> + return 0;
> +}


> +/* Read the last four bytes of the volume and make sure the AXFS magic is
> + present. */
> +int axfs_verify_eofs_magic(struct super_block *sb)
> +{
> + struct axfs_super *sbi = AXFS_SB(sb);
> + u32 buf = 0;
> + int err;
> + u32 fsoffset = sbi->size - sizeof(u32);
> + int len = sizeof(u32);
> +
> + err = axfs_copy_metadata(sb, &buf, fsoffset, len);

sizeof(buf) instead of an additional variable named len.

> +
> + if (err)
> + return -EINVAL;
> +
> + if (be32_to_cpu(buf) != AXFS_MAGIC) {
> + printk(KERN_ERR "READ: 0x%x\n", be32_to_cpu(buf));
> + printk(KERN_ERR "ERROR: Filesystem is incomplete and cannot be "
> + "mounted!\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}


> +int axfs_fill_super(struct super_block *sb, void *data, int silent)
> +{
> + struct axfs_super *sbi_in = (struct axfs_super *)data;
> + struct axfs_super *sbi;
> + struct inode *root;
> + int err;
> +
> + sbi = axfs_get_sbi();

missing check for failure

> + sb->s_fs_info = (void *)sbi;
> + memcpy(sbi, sbi_in, sizeof(*sbi));
> +
> + /* fully populate the incore superblock structures */
> + err = axfs_do_fill_super(sb);
> + if (err)
> + goto out;
> +
> + sb->s_flags |= MS_RDONLY;
> +
> + /* Setup the VFS super block now */
> + sb->s_op = &axfs_sops;
> + root = axfs_create_vfs_inode(sb, 0);
> + if (!root) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + sb->s_root = d_alloc_root(root);
> + if (!sb->s_root) {
> + iput(root);
> + err = -EINVAL;
> + goto out;
> + }
> +
> + err = axfs_init_profiling(sbi);
> + if (err)
> + goto out;
> +
> + return 0;
> +
> +out:
> + axfs_put_super(sb);
> + return err;
> +}


> +static struct axfs_super *axfs_get_sbi(void)
> +{
> + struct axfs_super *sbi;
> +
> + sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
> + if (sbi)
> + return sbi;
> +
> + axfs_put_sbi(sbi);

Looks useless, sbi is NULL here.

> + return ERR_PTR(-ENOMEM);
> +}


> +static int __init init_axfs_fs(void)
> +{
> + axfs_uncompress_init();
> + return register_filesystem(&axfs_fs_type);
> +}
> +
> +static void __exit exit_axfs_fs(void)
> +{
> + axfs_uncompress_exit();
> + unregister_filesystem(&axfs_fs_type);

Order looks wrong compared to init_axfs_fs()

> +}
> +
> +module_init(init_axfs_fs);
> +module_exit(exit_axfs_fs);
> +MODULE_LICENSE("GPL");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/