linux-kernel - Re: [PATCH 6/6] udf: fix sparse warnings (shadowing & mismatch between declaration and definition)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20071217165017.GJ6979@atrey.karlin.mff.cuni.cz>
Date:	Mon, 17 Dec 2007 17:50:17 +0100
From:	Jan Kara <jack@...e.cz>
To:	Marcin Slusarz <marcin.slusarz@...il.com>
Cc:	linux-kernel@...r.kernel.org,
	Ben Fennema <bfennema@...con.csc.calpoly.edu>
Subject: Re: [PATCH 6/6] udf: fix sparse warnings (shadowing & mismatch between declaration and definition)

> fix warnings:
> fs/udf/super.c:1320:24: warning: symbol 'bh' shadows an earlier one
> fs/udf/super.c:1240:21: originally declared here
> fs/udf/super.c:1583:4: warning: symbol 'i' shadows an earlier one
> fs/udf/super.c:1418:6: originally declared here
> fs/udf/super.c:1585:4: warning: symbol 'i' shadows an earlier one
> fs/udf/super.c:1418:6: originally declared here
> fs/udf/super.c:1658:4: warning: symbol 'i' shadows an earlier one
> fs/udf/super.c:1648:6: originally declared here
> fs/udf/super.c:1660:4: warning: symbol 'i' shadows an earlier one
> fs/udf/super.c:1648:6: originally declared here
> fs/udf/super.c:450:6: warning: symbol 'udf_write_super' was not declared. Should it be static?
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz@...il.com>
> CC: Ben Fennema <bfennema@...con.csc.calpoly.edu>
  Thanks for the patch.  The 'bh' change is fine. The problems with 'i'
should be solved differently I think. Those functions UDF_SB_FREE,
UDF_SB_ALLOC_PARTMAPS should be functions and not macros. Please convert
those to either inline functions if they are small or to regular
functions if they are larger.  It won't be completely trivial because of
the hackery e.g. in UDF_SB_ALLOC_BITMAP. It gets an argument meaning on
which struct member something should be performed. But for example in
the UDF_SB_ALLOC_BITMAP case you can simply make the function return the
pointer to allocated and initialized space and the caller would assign
it to a proper element of the superblock. This would help the overall
code quality of UDF (which is sadly quite poor). Thanks.

								Honza
> ---
>  fs/udf/super.c |   15 +++++++--------
>  1 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 4360c7a..9f82b5a 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -447,7 +447,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt)
>  	return 1;
>  }
> 
> -void udf_write_super(struct super_block *sb)
> +static void udf_write_super(struct super_block *sb)
>  {
>  	lock_kernel();
> 
> @@ -1317,7 +1317,6 @@ static int udf_load_partition(struct super_block *sb, kernel_lb_addr *fileset)
>  				UDF_SB_TYPEVIRT(sb, i).s_num_entries =
>  					(UDF_SB_VAT(sb)->i_size - 36) >> 2;
>  			} else if (UDF_SB_PARTTYPE(sb, i) == UDF_VIRTUAL_MAP20) {
> -				struct buffer_head *bh = NULL;
>  				uint32_t pos;
> 
>  				pos = udf_block_map(UDF_SB_VAT(sb), 0);
> @@ -1415,7 +1414,7 @@ static void udf_close_lvid(struct super_block *sb)
>   */
>  static int udf_fill_super(struct super_block *sb, void *options, int silent)
>  {
> -	int i;
> +	int idx;
>  	struct inode *inode = NULL;
>  	struct udf_options uopt;
>  	kernel_lb_addr rootdir, fileset;
> @@ -1584,8 +1583,8 @@ error_out:
>  		if (UDF_SB_PARTFLAGS(sb, UDF_SB_PARTITION(sb)) & UDF_PART_FLAG_FREED_BITMAP)
>  			UDF_SB_FREE_BITMAP(sb,UDF_SB_PARTITION(sb), s_fspace);
>  		if (UDF_SB_PARTTYPE(sb, UDF_SB_PARTITION(sb)) == UDF_SPARABLE_MAP15) {
> -			for (i = 0; i < 4; i++)
> -				brelse(UDF_SB_TYPESPAR(sb, UDF_SB_PARTITION(sb)).s_spar_map[i]);
> +			for (idx = 0; idx < 4; idx++)
> +				brelse(UDF_SB_TYPESPAR(sb, UDF_SB_PARTITION(sb)).s_spar_map[idx]);
>  		}
>  	}
>  #ifdef CONFIG_UDF_NLS
> @@ -1645,7 +1644,7 @@ void udf_warning(struct super_block *sb, const char *function,
>   */
>  static void udf_put_super(struct super_block *sb)
>  {
> -	int i;
> +	int idx;
> 
>  	if (UDF_SB_VAT(sb))
>  		iput(UDF_SB_VAT(sb));
> @@ -1659,8 +1658,8 @@ static void udf_put_super(struct super_block *sb)
>  		if (UDF_SB_PARTFLAGS(sb, UDF_SB_PARTITION(sb)) & UDF_PART_FLAG_FREED_BITMAP)
>  			UDF_SB_FREE_BITMAP(sb,UDF_SB_PARTITION(sb), s_fspace);
>  		if (UDF_SB_PARTTYPE(sb, UDF_SB_PARTITION(sb)) == UDF_SPARABLE_MAP15) {
> -			for (i = 0; i < 4; i++)
> -				brelse(UDF_SB_TYPESPAR(sb, UDF_SB_PARTITION(sb)).s_spar_map[i]);
> +			for (idx = 0; idx < 4; idx++)
> +				brelse(UDF_SB_TYPESPAR(sb, UDF_SB_PARTITION(sb)).s_spar_map[idx]);
>  		}
>  	}
>  #ifdef CONFIG_UDF_NLS
> --
> 1.5.3.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
Jan Kara <jack@...e.cz>
SuSE CR Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/