SVN compilation fix: fs/fat.c

[Uwe Bonnes]

Hello,

with (avr-)gcc version 4.4.1 [gcc-4_4-branch revision 150839] (SUSE Linux)
recent Sourceforge SVN doesn't compile nut/fs/fat.c:
cc1: warnings being treated as errors
../.././/fs/fat.c: In function 'MountATAPI':
../.././/fs/fat.c:934: error: dereferencing type-punned pointer will break strict-aliasing rules
../.././/fs/fat.c:935: error: dereferencing type-punned pointer will break strict-aliasing rules

Appended patch defines the structure with the types later used.

Bye
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
Index: nut/fs/fat.c
===================================================================
--- nut/fs/fat.c (Revision 2740)
+++ nut/fs/fat.c (Arbeitskopie)
@@ -367,8 +367,8 @@
     BYTE volseqnum[4];
     BYTE seksize[4];
     BYTE pathtablen[8];
-    BYTE firstsek_LEpathtab1_LE[4];
-    BYTE firstsek_LEpathtab2_LE[4];
+    DWORD firstsek_LEpathtab1_LE;
+    DWORD firstsek_LEpathtab2_LE;
     BYTE firstsek_BEpathtab1_BE[4];
     BYTE firstsek_BEpathtab2_BE[4];
     BYTE rootdir[34];
@@ -931,8 +931,8 @@
 
         pDrive->bSectorsPerCluster = 1;
         pDrive->dwFirstRootDirSector = *(DWORD *) & pPVD->rootdir[2];
-        pDrive->dwFAT1StartSector = *(DWORD *) & pPVD->firstsek_LEpathtab1_LE[0];
-        pDrive->dwFAT2StartSector = *(DWORD *) & pPVD->firstsek_LEpathtab2_LE[0];
+        pDrive->dwFAT1StartSector = pPVD->firstsek_LEpathtab1_LE;
+        pDrive->dwFAT2StartSector = pPVD->firstsek_LEpathtab2_LE;
 
         pDrive->dwRootDirSectors = pDrive->dwFAT1StartSector - pDrive->dwFirstRootDirSector;
         pDrive->dwRootCluster = pDrive->dwFirstRootDirSector;
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Thiago A. Corrêa]

Hi,

On Mon, Oct 5, 2009 at 1:35 PM, Uwe Bonnes
<bon@...> wrote:
> -    BYTE firstsek_LEpathtab1_LE[4];
> -    BYTE firstsek_LEpathtab2_LE[4];
> +    DWORD firstsek_LEpathtab1_LE;
> +    DWORD firstsek_LEpathtab2_LE;

I don't currently use this driver, but looks reasonable. Perhaps we
should actually change all BYTE to uint8_t and all DWORD to uint32_t
like the rest of the Nut/OS. The _t types are quite carefully defined,
can't say if those upper letter types are.

Kind Regards,
    Thiago A. Correa
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Uwe Bonnes]

>>>>> "Thiago" == Thiago A Corrêa <thiago.correa@...> writes:

    Thiago> Hi, On Mon, Oct 5, 2009 at 1:35 PM, Uwe Bonnes
    Thiago> <bon@...> wrote:
    >> -    BYTE firstsek_LEpathtab1_LE[4]; -    BYTE
    >> firstsek_LEpathtab2_LE[4]; +    DWORD firstsek_LEpathtab1_LE; +  
    >>  DWORD firstsek_LEpathtab2_LE;

    Thiago> I don't currently use this driver, but looks reasonable. Perhaps
    Thiago> we should actually change all BYTE to uint8_t and all DWORD to
    Thiago> uint32_t like the rest of the Nut/OS. The _t types are quite
    Thiago> carefully defined, can't say if those upper letter types are.

Should I provide a patch?
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Thiago A. Corrêa]

Hi,

On Mon, Oct 5, 2009 at 2:14 PM, Uwe Bonnes
<bon@...> wrote:

>>>>>> "Thiago" == Thiago A Corrêa <thiago.correa@...> writes:
>
>    Thiago> Hi, On Mon, Oct 5, 2009 at 1:35 PM, Uwe Bonnes
>    Thiago> <bon@...> wrote:
>    >> -    BYTE firstsek_LEpathtab1_LE[4]; -    BYTE
>    >> firstsek_LEpathtab2_LE[4]; +    DWORD firstsek_LEpathtab1_LE; +
>    >>  DWORD firstsek_LEpathtab2_LE;
>
>    Thiago> I don't currently use this driver, but looks reasonable. Perhaps
>    Thiago> we should actually change all BYTE to uint8_t and all DWORD to
>    Thiago> uint32_t like the rest of the Nut/OS. The _t types are quite
>    Thiago> carefully defined, can't say if those upper letter types are.
>
> Should I provide a patch?

It would be great, but I think we should hear if Harald is Ok with it
first. I guess he will be, but as I don't maintain this driver, it's
best to be sure. It's possible that he forgot when we moved to those
uint32_t or perhaps there is a reason to keep it like that.

Kind Regards,
    Thiago A. Correa
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Ethernut]

Thiago A. Corrêa wrote:

> On Mon, Oct 5, 2009 at 2:14 PM, Uwe Bonnes wrote:
>>>>>>> "Thiago" == Thiago A Corrêa  writes:
>>    Thiago> Hi, On Mon, Oct 5, 2009 at 1:35 PM, Uwe Bonnes
>>    Thiago>  wrote:
>>    >> -    BYTE firstsek_LEpathtab1_LE[4]; -    BYTE
>>    >> firstsek_LEpathtab2_LE[4]; +    DWORD firstsek_LEpathtab1_LE; +
>>    >>  DWORD firstsek_LEpathtab2_LE;
>>
>>    Thiago> I don't currently use this driver, but looks reasonable. Perhaps
>>    Thiago> we should actually change all BYTE to uint8_t and all DWORD to
>>    Thiago> uint32_t like the rest of the Nut/OS. The _t types are quite
>>    Thiago> carefully defined, can't say if those upper letter types are.
>>
>> Should I provide a patch?
>
> It would be great, but I think we should hear if Harald is Ok with it
> first. I guess he will be, but as I don't maintain this driver, it's
> best to be sure. It's possible that he forgot when we moved to those
> uint32_t or perhaps there is a reason to keep it like that.

I don't maintain it either. Actually nobody is looking after it. It has
been initially contributed by Michael Fischer. Check the "Nut/OS File
System History" at

http://www.ethernut.de/en/documents/phat.html

I assume, that Michael is too busy with other things and will probably
welcome any effort to update this driver.

Harald

_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[oetelaar]

Harald Kipp wrote:

> Thiago A. Corrêa wrote:
>  
>> On Mon, Oct 5, 2009 at 2:14 PM, Uwe Bonnes wrote:
>>    
>>>>>>>> "Thiago" == Thiago A Corrêa  writes:
>>>>>>>>                
>>>    Thiago> Hi, On Mon, Oct 5, 2009 at 1:35 PM, Uwe Bonnes
>>>    Thiago>  wrote:
>>>    >> -    BYTE firstsek_LEpathtab1_LE[4]; -    BYTE
>>>    >> firstsek_LEpathtab2_LE[4]; +    DWORD firstsek_LEpathtab1_LE; +
>>>    >>  DWORD firstsek_LEpathtab2_LE;
>>>
>>>    Thiago> I don't currently use this driver, but looks reasonable. Perhaps
>>>    Thiago> we should actually change all BYTE to uint8_t and all DWORD to
>>>    Thiago> uint32_t like the rest of the Nut/OS. The _t types are quite
>>>    Thiago> carefully defined, can't say if those upper letter types are.
>>>
>>> Should I provide a patch?
>>>      
>> It would be great, but I think we should hear if Harald is Ok with it
>> first. I guess he will be, but as I don't maintain this driver, it's
>> best to be sure. It's possible that he forgot when we moved to those
>> uint32_t or perhaps there is a reason to keep it like that.
>>    
>
> I don't maintain it either. Actually nobody is looking after it. It has
> been initially contributed by Michael Fischer. Check the "Nut/OS File
> System History" at
>
> http://www.ethernut.de/en/documents/phat.html
>
> I assume, that Michael is too busy with other things and will probably
> welcome any effort to update this driver.
>
> Harald

any chance of hacking the new exFat into this ?
Just asking because people start using Vista to format SD cards and the
first complaints are coming.
http://www.microsoft.com/iplicensing/productDetail.aspx?productTitle=exFAT%20File%20System%20Licensing%20Program

Regards,
Edwin


_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Uwe Bonnes]

>>>>> "Harald" == Harald Kipp <harald.kipp@...> writes:
...
    Harald> I assume, that Michael is too busy with other things and will
    Harald> probably welcome any effort to update this driver.

I'll try to find time and use stdint.h types. As it seems nobody is working
on the file, if the changes are too big. I'll send the whole file and you
can substitute in your tree, checking in the file  afterwards.

Bye
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion

[Uwe Bonnes]

>>>>> "Thiago" == Thiago A Corrêa <thiago.correa@...> writes:

    Thiago> Hi, On Mon, Oct 5, 2009 at 1:35 PM, Uwe Bonnes
    Thiago> <bon@...> wrote:
    >> -    BYTE firstsek_LEpathtab1_LE[4]; -    BYTE
    >> firstsek_LEpathtab2_LE[4]; +    DWORD firstsek_LEpathtab1_LE; +  
    >>  DWORD firstsek_LEpathtab2_LE;

    Thiago> I don't currently use this driver, but looks reasonable. Perhaps
    Thiago> we should actually change all BYTE to uint8_t and all DWORD to
    Thiago> uint32_t like the rest of the Nut/OS. The _t types are quite
    Thiago> carefully defined, can't say if those upper letter types are.

Well, the "rest of Nut/OS" still also isn't free of BYTE/WORD/DWORD etc,
especially in the AVR tree

> find nut/ -name \*.c | xargs grep " BYTE " | cut -d ':' -f 1 | sort | uniq
nut/arch/avr/dev/ide.c
nut/arch/avr/dev/pcmcia.c
nut/arch/avr/dev/wlandrv.c
nut/fs/fat.c

So I vote to apply my initial patch first, and then to clean up the BYTE etc
situation in another step.

Bye
--
Uwe Bonnes                bon@...

Institut fuer Kernphysik  Schlossgartenstrasse 9  64289 Darmstadt
--------- Tel. [phone] -------- Fax. [phone] ----------
_______________________________________________
http://lists.egnite.de/mailman/listinfo/en-nut-discussion