Patchwork [U-Boot] fs/fat/fat.c: fix warning: 'part_size' defined but not used

login
register
mail settings
Submitter Wolfgang Denk
Date Oct. 27, 2011, 9:10 p.m.
Message ID <1319749858-29567-1-git-send-email-wd@denx.de>
Download mbox | patch
Permalink /patch/122249/
State Accepted
Commit e116cc069fa4cbe51f3dab84698bc0ac57085f62
Headers show

Comments

Wolfgang Denk - Oct. 27, 2011, 9:10 p.m.
Commit c30a15e "FAT: Add FAT write feature" introduced a compiler
warning.  Fix this.

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Donggeun Kim <dg77.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
---

 fs/fat/fat.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
Aaron Williams - Dec. 13, 2011, 6:15 a.m.
On Thursday, October 27, 2011 02:10:58 PM Wolfgang Denk wrote:
> Commit c30a15e "FAT: Add FAT write feature" introduced a compiler
> warning.  Fix this.
> 
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Donggeun Kim <dg77.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> 
>  fs/fat/fat.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index 7cf173c..756ac64 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -46,7 +46,6 @@ static void downcase (char *str)
>  static block_dev_desc_t *cur_dev = NULL;
> 
>  static unsigned long part_offset = 0;
> -static unsigned long part_size;
> 
>  static int cur_part = 1;
> 
> @@ -100,7 +99,6 @@ int fat_register_device (block_dev_desc_t * dev_desc,
> int part_no) if (!get_partition_info(dev_desc, part_no, &info)) {
>  		part_offset = info.start;
>  		cur_part = part_no;
> -		part_size = info.size;
>  	} else if ((strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3) == 0)
> || (strncmp((char *)&buffer[DOS_FS32_TYPE_OFFSET], "FAT32", 5) == 0)) { /*
> ok, we assume we are on a PBR only */

Hi Wolfgang,

I  know it's rather late to comment on this, but this patch breaks FAT write 
support.

-Aaron
Wolfgang Denk - Dec. 13, 2011, 8:18 a.m.
Dear Aaron Williams,

In message <201112122215.11610.Aaron.Williams@cavium.com> you wrote:
> 
> > Commit c30a15e "FAT: Add FAT write feature" introduced a compiler
> > warning.  Fix this.
...
> I  know it's rather late to comment on this, but this patch breaks FAT write 
> support.

How can it break something we don't have?

Currently there is no write support for (V)FAT file systems in
mainline.

The commit removes a static variable of file scope, that was used in a
single place, where a value was assigned to it.  There was no place
anywhere in the code twhere else this variable was referenced.  So how
can this break anything?

Please elucidate.

Best regards,

Wolfgang Denk
Donggeun Kim - Dec. 13, 2011, 8:37 a.m.
On 2011년 12월 13일 15:15, Aaron Williams wrote:
> On Thursday, October 27, 2011 02:10:58 PM Wolfgang Denk wrote:
>> Commit c30a15e "FAT: Add FAT write feature" introduced a compiler
>> warning.  Fix this.
>>
>> Signed-off-by: Wolfgang Denk <wd@denx.de>
>> Cc: Donggeun Kim <dg77.kim@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>
>>  fs/fat/fat.c |    2 --
>>  1 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index 7cf173c..756ac64 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -46,7 +46,6 @@ static void downcase (char *str)
>>  static block_dev_desc_t *cur_dev = NULL;
>>
>>  static unsigned long part_offset = 0;
>> -static unsigned long part_size;
>>
>>  static int cur_part = 1;
>>
>> @@ -100,7 +99,6 @@ int fat_register_device (block_dev_desc_t * dev_desc,
>> int part_no) if (!get_partition_info(dev_desc, part_no, &info)) {
>>  		part_offset = info.start;
>>  		cur_part = part_no;
>> -		part_size = info.size;
>>  	} else if ((strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3) == 0)
>> || (strncmp((char *)&buffer[DOS_FS32_TYPE_OFFSET], "FAT32", 5) == 0)) { /*
>> ok, we assume we are on a PBR only */
> 
> Hi Wolfgang,
> 
> I  know it's rather late to comment on this, but this patch breaks FAT write 
> support.
> 
> -Aaron
> 
> 
Hi, Wolfgang,

I've overlooked the patch before.
If 'part_size' variable is not set in fat_register_device function, FAT
write feature would not work properly.
So, I will send a patch to define the variable only when FAT_WRITE is
enabled. It can avoid compile warning.

Thanks.
-Donggeun
Anatolij Gustschin - Dec. 13, 2011, 8:53 a.m.
Hello all,

On Tue, 13 Dec 2011 09:18:15 +0100
Wolfgang Denk <wd@denx.de> wrote:

> Dear Aaron Williams,
> 
> In message <201112122215.11610.Aaron.Williams@cavium.com> you wrote:
> > 
> > > Commit c30a15e "FAT: Add FAT write feature" introduced a compiler
> > > warning.  Fix this.
> ...
> > I  know it's rather late to comment on this, but this patch breaks FAT write 
> > support.
> 
> How can it break something we don't have?
> 
> Currently there is no write support for (V)FAT file systems in
> mainline.
> 
> The commit removes a static variable of file scope, that was used in a
> single place, where a value was assigned to it.  There was no place
> anywhere in the code twhere else this variable was referenced.  So how
> can this break anything?
> 
> Please elucidate.

The FAT write support as submitted and included seems to be broken.
The removed variable is referenced in fs/fat/fat_write.c, but in
fs/fat/fat.c it is declared as static. This issue didn't show up
because no board config file in mainline defines CONFIG_FAT_WRITE.

@ Donggeun Kim
Could you please submit a patch fixing this?

Thanks,
Anatolij
Wolfgang Denk - Dec. 13, 2011, 11:03 a.m.
Dear Anatolij Gustschin,

In message <20111213095358.5601b9ce@wker> you wrote:
> 
> The FAT write support as submitted and included seems to be broken.
> The removed variable is referenced in fs/fat/fat_write.c, but in
> fs/fat/fat.c it is declared as static. This issue didn't show up
> because no board config file in mainline defines CONFIG_FAT_WRITE.

It's not only that.  It appears that this code is also not hooked upo
to any command a user could call.  At least, I cannot see anything
like that in common/cmd_fat.c or so...

> @ Donggeun Kim
> Could you please submit a patch fixing this?

I guess this needs more fixing...

Best regards,

Wolfgang Denk

Patch

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 7cf173c..756ac64 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -46,7 +46,6 @@  static void downcase (char *str)
 static block_dev_desc_t *cur_dev = NULL;
 
 static unsigned long part_offset = 0;
-static unsigned long part_size;
 
 static int cur_part = 1;
 
@@ -100,7 +99,6 @@  int fat_register_device (block_dev_desc_t * dev_desc, int part_no)
 	if (!get_partition_info(dev_desc, part_no, &info)) {
 		part_offset = info.start;
 		cur_part = part_no;
-		part_size = info.size;
 	} else if ((strncmp((char *)&buffer[DOS_FS_TYPE_OFFSET], "FAT", 3) == 0) ||
 		   (strncmp((char *)&buffer[DOS_FS32_TYPE_OFFSET], "FAT32", 5) == 0)) {
 		/* ok, we assume we are on a PBR only */