Messages in this thread |  | | Date | Tue, 25 Dec 2007 11:59:46 +0000 | | From | Christoph Hellwig <> | | Subject | Re: [PATCH 3/6] udf: convert UDF_SB_ALLOC_PARTMAPS macro to udf_sb_alloc_partition_maps function |
| |
On Mon, Dec 24, 2007 at 01:10:19AM +0100, marcin.slusarz@gmail.com wrote: > @@ -53,6 +53,7 @@ > #include <linux/vfs.h> > #include <linux/vmalloc.h> > #include <asm/byteorder.h> > +#include <linux/errno.h>
linux/*.h always before asm/*.h
> +static int __must_check udf_sb_alloc_partition_maps(struct super_block *sb, __u32 count)
I think __must_check is primarily for public APIs. But if it really helps you here I won't complain.
Also please use u32 over __u32 (dito for the other similar types)
> +{ > + struct udf_sb_info *sbi = UDF_SB(sb); > + sbi->s_partmaps = kzalloc(sizeof(struct udf_part_map) * count, GFP_KERNEL); > + if (sbi->s_partmaps != NULL) > + sbi->s_partitions = count; > + else { > + sbi->s_partitions = 0; > + udf_error(sb, __FUNCTION__, "Unable to allocate space for %d partition maps", count); > + } > + return sbi->s_partmaps != NULL ? 0 : -ENOMEM; > +}
But please don't introduce new overlong lines in entirely new functions.
Also the code look rather odd, I'd rather write it as:
static int udf_sb_alloc_partition_maps(struct super_block *sb, u32 count) { struct udf_sb_info *sbi = UDF_SB(sb);
sbi->s_partmaps = kcalloc(count, sizeof(struct udf_part_map), GFP_KERNEL); if (!sbi->s_partmaps) { udf_error(sb, __FUNCTION__, "Unable to allocate space for %d partition maps", count); sbi->s_partitions = 0; return -ENOMEM; }
sbi->s_partitions = count; return 0; }
|  |