On Tuesday 27 December 2011 21:23:27 uma.shankar@samsung.com wrote:
From: Uma Shankar uma.shankar@samsung.com
Signed-off-by: Uma Shankar uma.shankar@samsung.com Signed-off-by: Manjunatha C Achar a.manjunatha@samsung.com Signed-off-by: Iqbal Shareef iqbal.ams@samsung.com Signed-off-by: Hakgoo Lee goodguy.lee@samsung.com
you need to note the sources of these files in the changelog
--- a/common/cmd_ext4.c +++ b/common/cmd_ext4.c
+int do_ext4_write(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
static
- char *filename = "/";
const
- int dev = 0;
considering you set dev first below, no point in setting this to 0
- /*get the filename */
many of the comments here are missing a space after the "/*"
--- /dev/null +++ b/fs/ext4/README
docs belong in docs/
--- /dev/null +++ b/fs/ext4/crc16.c
+/*
crc16.c
- This source code is licensed under the GNU General Public License,
- Version 2. See the file COPYING for more details.
- */
why can't you use the existing lib/crc16.c code ?
--- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c
+#if defined(CONFIG_CMD_EXT4_WRITE) +#include "ext4_journal.h" +#include "crc16.h" +#else #include "ext4_common.h" +#endif
conditional header includes are a bad idea
+/* To convert big endian journal superblock entries to little endian */ +unsigned int be_le(unsigned int num) ... +/* 4-byte number */ +unsigned int le_be(unsigned int num)
we already have swap functions in include/ you should be able to use instead of duplicating them
+void reset_inode_bmap(int inode_no, unsigned char *buffer, int index) ...
- return;
+}
useless return -> delete it
--- a/fs/ext4/ext4_common.h +++ b/fs/ext4/ext4_common.h
+int get_parent_inode_num(char *dirname, char *dname, int flags); +void update_parent_dentry(char *filename, int *p_ino, int file_type); +long int get_new_blk_no(void); +int get_new_inode_no(void); +void reset_block_bmap(long int blockno, unsigned char *buffer, int index); +int set_block_bmap(long int blockno, unsigned char *buffer, int index); +int set_inode_bmap(int inode_no, unsigned char *buffer, int index); +void reset_inode_bmap(int inode_no, unsigned char *buffer, int index); +int iget(int inode_no, struct ext2_inode *inode); +void allocate_blocks(struct ext2_inode *file_inode,
unsigned int total_remaining_blocks,unsigned int *total_no_of_block);+void put_ext4(uint64_t off, void *buf, uint32_t size);
--- /dev/null +++ b/fs/ext4/ext4_journal.h
+int init_journal(void); +void deinit_journal(void); +int log_gdt(char *gd_table); +void update_journal(void); +int check_journal_state(int recovery_flag); +int log_journal(char *journal_buffer, long int blknr); +int put_metadata(char *metadata_buffer, long int blknr); +void dump_metadata(void); +void free_journal(void); +void free_revoke_blks(void); +void push_revoke_blk(char *buffer);
these are a lot of bad names. these need to be properly namespaced if you're going to export them.
--- /dev/null +++ b/fs/ext4/ext4_journal.c
journal_ptr[i] = (struct journal_log *)zalloc(sizeof(struct journal_log));...
dirty_block_ptr[i] = (struct dirty_blocks *)zalloc(sizeof(struct dirty_blocks));...
journal_ptr[gindex]->buf = (char *)zalloc(fs->blksz);...
journal_ptr[gindex]->buf = (char *)zalloc(fs->blksz);
useless casts. zalloc() returns void *. there's a bunch more throughout this patch.
+void free_journal(void) +{ ...
- return;
+}
useless return
+void print_revoke_blks(char *revk_blk) +{ ...
- return;
+}
useless return
+static struct revoke_blk_list *_get_node(void) +{ ...
- return;
+}
useless return
+void free_revoke_blks()
needs (void)
- return;
+}
useless return
+void print_jrnl_status(int recovery_flag) +{ ...
- return;
+}
useless return
--- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c
+static void delete_single_indirect_block(struct ext2_inode *inode) +{ ...
- return;
+}
useless return
+static void delete_double_indirect_block(struct ext2_inode *inode) +{ ...
- return;
+}
useless return
+static void delete_triple_indirect_block(struct ext2_inode *inode) +{ ...
- return;
+}
useless return
+void ext4fs_deinit(void) +{ ...
- return;
+}
useless return -mike