[Jan Kara - Thu, May 24, 2007 at 10:37:14AM +0200]
| On Wed 23-05-07 22:44:36, Cyrill Gorcunov wrote:
| > This patch converts UDF system coding style to kernel coding style.
| > There is no error fixing - just code cleanup. I attempted to make
| > the minimum changes as possible but the patch was growing and growing.
| > So there is a lot of code to review and that could be painful and boring.
| > I've it reviewed several times to assure that the patch does not break
| > current UDF system... but I've only one pair of eyes.
| >
| > Any comments, suggestions and swearing are welcome. But don't be
| > expected for fast reply - I'm not mature kernel developer ;)
| >
| > Signed-off-by: Cyrill Gorcunov <[EMAIL PROTECTED]>
| Thanks for doing this tedious work... One general comment: Kernel is
| supposed to be viewable on a terminal 80 characters wide so you should
| avoid making longer lines whenever possible (I agree that there are
| cases where wrapped line looks worse that a long line but that's
| just my personal opinion ;).
|
Jan, I think the problem is about long UDF structures names.
The only purpose for this patch is to set up braces and spaces
properly. I'll make lines shorter as soon as _this_ patch become
veryfied and included in main kernel. Moreover I also should
get rid of macroses and other stuff. Lets do work by small steps ;)
But you are absolutely right!
| > static void udf_merge_extents(struct inode *inode,
| > - kernel_long_ad laarr[EXTENT_MERGE_SIZE], int *endnum)
| > + kernel_long_ad laarr[EXTENT_MERGE_SIZE],
| > + int *endnum)
| > {
| > int i;
| >
| > - for (i=0; i<(*endnum-1); i++)
| > + for (i = 0; i < (*endnum - 1); i++)
| > {
| ^^^^^^^ the brace should be on the previous line
Thanks for note. I've it just missed.
|
| > @@ -1283,86 +1201,74 @@ static void udf_fill_inode(struct inode *inode,
struct buffer_head *bh)
| > offset = sizeof(struct extendedFileEntry) +
UDF_I_LENEATTR(inode);
| > }
| >
| > - switch (fe->icbTag.fileType)
| > - {
| > + switch (fe->icbTag.fileType) {
| > case ICBTAG_FILE_TYPE_DIRECTORY:
| > - {
| > - inode->i_op = &udf_dir_inode_operations;
| > + inode->i_op = (struct inode_operations
*)&udf_dir_inode_operations;
| ^^^ Why have you added explicit retyping?
This became from my local git branch. Thanks. Will be removed.
|
| > inode->i_fop = &udf_dir_operations;
| > inode->i_mode |= S_IFDIR;
| > inc_nlink(inode);
| > break;
| > - }
| > case ICBTAG_FILE_TYPE_REALTIME:
| > case ICBTAG_FILE_TYPE_REGULAR:
| > case ICBTAG_FILE_TYPE_UNDEF:
| > - {
| > if (UDF_I_ALLOCTYPE(inode) == ICBTAG_FLAG_AD_IN_ICB)
| > inode->i_data.a_ops = &udf_adinicb_aops;
| > else
| > inode->i_data.a_ops = &udf_aops;
| > - inode->i_op = &udf_file_inode_operations;
| > + inode->i_op = (struct inode_operations
*)&udf_file_inode_operations;
| ^^^ And here again?
|
|
Arh!
| > -static mode_t
| > -udf_convert_permissions(struct fileEntry *fe)
| > +static int udf_alloc_i_data(struct inode *inode, size_t size)
| > +{
| > + UDF_I_DATA(inode) = kmalloc(size, GFP_KERNEL);
| > +
| > + if (!UDF_I_DATA(inode)) {
| > + printk(KERN_ERR "udf:udf_alloc_i_data (ino %ld) no free
memory\n",
| > + inode->i_ino);
| > + return -ENOMEM;
| > + }
| > +
| > + return 0;
| > +}
| > +
| It seems you've introduced a new function but never used it...
Jan I've messed up with this patch and my prev patches that already
in -mm tree. Actually udf_alloc_i_data was introduced by my prev patch.
I'll fix this mess.
|
|
| > @@ -1573,35 +1465,30 @@ static int udf_fill_super(struct super_block *sb,
void *options, int silent)
| > udf_find_anchor(sb);
| >
| > /* Fill in the rest of the superblock */
| > - sb->s_op = &udf_sb_ops;
| > + sb->s_op = (struct super_operations *)&udf_sb_ops;
| Again this explicit typing shouldn't be needed...
Damn, sorry.
|
|
Honza
| --
| Jan Kara <[EMAIL PROTECTED]>
| SuSE CR Labs
|
Andew please drop this patch. I'll make new one (over -mm tree ;)
Cyrill
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
