Re: [PATCH] mmc: card: move variable initialization earlier

[Chris Ball <cjb@laptop.org>]

Hi Adrian,

On Fri, Mar 23 2012, Linus Walleij wrote:
> I was pretty tired of seeing these in my kernel compiles:
>
> drivers/mmc/card/block.c: In function ‘mmc_blk_issue_secdiscard_rq’:
> drivers/mmc/card/block.c:911:18: warning: ‘arg’ may be used uninitialized in this function [-Wuninitialized]
> drivers/mmc/card/block.c:910:6: warning: ‘nr’ may be used uninitialized in this function [-Wuninitialized]
> drivers/mmc/card/block.c:910:6: warning: ‘from’ may be used uninitialized in this function [-Wuninitialized]
>
> The problem stems from the code path in
> mmc_blk_issue_secdiscard_rq() where mmc_switch()
> with EXT_CSD_SANITIZE_START may return -EIO and fall back
> to using the default trim operations instead. At this point
> the variables needed for the fallback will be uninitialized.
>
> Cc: Ulf Hansson <ulf.hansson@stericsson.com>
> Cc: Rabin Vincent <rabin@rab.in>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I don't know if this is the actual intention - maybe we
> should just fail the call entirely if the sanitize command
> fails?

I think you (Adrian) introduced this "goto out->goto retry" logic in
upstream commit 67716327eec7e9 -- please could you take a look here?

> ---
>  drivers/mmc/card/block.c |   15 ++++++++-------
>  1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index c6a383d..2741d93 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -877,6 +877,14 @@ static int mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
>  	unsigned int from, nr, arg;
>  	int err = 0, type = MMC_BLK_SECDISCARD;
>  
> +	from = blk_rq_pos(req);
> +	nr = blk_rq_sectors(req);
> +
> +	if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
> +		arg = MMC_SECURE_TRIM1_ARG;
> +	else
> +		arg = MMC_SECURE_ERASE_ARG;
> +
>  	if (!(mmc_can_secure_erase_trim(card) || mmc_can_sanitize(card))) {
>  		err = -EOPNOTSUPP;
>  		goto out;
> @@ -889,13 +897,6 @@ static int mmc_blk_issue_secdiscard_rq(struct mmc_queue *mq,
>  		goto out;
>  	}
>  
> -	from = blk_rq_pos(req);
> -	nr = blk_rq_sectors(req);
> -
> -	if (mmc_can_trim(card) && !mmc_erase_group_aligned(card, from, nr))
> -		arg = MMC_SECURE_TRIM1_ARG;
> -	else
> -		arg = MMC_SECURE_ERASE_ARG;
>  retry:
>  	if (card->quirks & MMC_QUIRK_INAND_CMD38) {
>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child