On Tuesday 27 December 2011 21:23:27 uma.shan...@samsung.com wrote: > From: Uma Shankar <uma.shan...@samsung.com> > > Signed-off-by: Uma Shankar <uma.shan...@samsung.com> > Signed-off-by: Manjunatha C Achar <a.manjuna...@samsung.com> > Signed-off-by: Iqbal Shareef <iqbal....@samsung.com> > Signed-off-by: Hakgoo Lee <goodguy....@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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
