0

I have a section of code that sometimes produces correct results. The only difference is I am storing the result in an intermediate variable.

The overall function signature is:

FS_FATEntry getFATEntryForCluster(FS_Cluster cluster, FS_Instance * fsi)

This works:

switch (fsi->type) {
    case FS_FAT12:
        if (cluster % 2)
            return ((*((uint16_t *)&FATSector[entOffset])) >> 4);
        else
            return ((*((uint16_t *)&FATSector[entOffset])) & 0x0FFF);
    case FS_FAT16:
        return (*((uint16_t *)&FATSector[entOffset]));
    case FS_FAT32:
        return ((*((uint32_t *)&FATSector[entOffset])) & 0x0FFFFFFF);
}

This does not: (note that FS_FATEntry is typedef'd as a uint32_t)

FS_FATEntry entry = 0xFFFFFFFF;
switch (fsi->type) {
    case FS_FAT12:
        if (cluster % 2)
            entry = ((*((uint16_t *)&FATSector[entOffset])) >> 4);
        else
            entry = ((*((uint16_t *)&FATSector[entOffset])) & 0x0FFF);
    case FS_FAT16:
        entry = (*((uint16_t *)&FATSector[entOffset]));
    case FS_FAT32:
        entry = ((*((uint32_t *)&FATSector[entOffset])) & 0x0FFFFFFF);
}
free(FATSector);
printf("Cluster %04X : %04X Entry\n", cluster, entry);
return entry;

When running the second piece of code, this is a snip of the output I get:

Cluster 00F9 : FB0FA0 Entry
Cluster 00FA : D0FC0FB Entry
Cluster 00FB : FD0FC0 Entry
Cluster 00FC : F0FE0FD Entry
Cluster 00FD : FF0FE0 Entry
Cluster 00FE : 11000FF Entry
Cluster 00FF : 1011000 Entry
Cluster 0100 : 3102101 Entry
Cluster 0101 : 1031021 Entry
Cluster 0102 : 5104103 Entry
Cluster 0103 : 1051041 Entry
Cluster 0104 : 7FFF105 Entry
Cluster 0105 : 107FFF1 Entry
Cluster 0106 : FFFF107 Entry
Cluster 0107 : FFFFFF1 Entry
Cluster 0108 : B10AFFF Entry
Cluster 0109 : 10B10AF Entry
Cluster 010A : D10C10B Entry
Cluster 010B : 10D10C1 Entry
Cluster 010C : F10E10D Entry
Cluster 010D : FFF10E1 Entry
Cluster 010E : 1110FFF Entry
Cluster 010F : 111110F Entry
Erik Johnson
  • 858
  • 1
  • 7
  • 29
  • 3
    "this is a snip of the output". You need to do more than just dump the output. You need to explain what it is about the output you want us to notice. I'm guessing it's wrong in some way. So please tell us what the actual output is supposed to be. And you really should provide a [minimal complete and verifiable](https://stackoverflow.com/help/mcve) version of your code. – kaylum Apr 06 '16 at 03:31
  • @kaylum I will add the expected output. A MWE would be extremely difficult since it requires many files and a FAT12 disk image – Erik Johnson Apr 06 '16 at 03:41
  • Probably no need. Looks like you already have a perfectly good answer. But do note for improving future questions. – kaylum Apr 06 '16 at 03:42
  • All of the `*(T *)` violate the strict aliasing rule – M.M Apr 06 '16 at 03:42
  • @M.M could you elaborate? This is how it is given in the Microsoft Whitepaper for FAT12/16/32 – Erik Johnson Apr 06 '16 at 03:44
  • If so, it'd be relying on nonstandard behaviour of the MS compiler – M.M Apr 06 '16 at 03:54
  • @M.M since this works without errors on GCC on Debian, can you explain what the exact problem is with this code? – Erik Johnson Apr 06 '16 at 13:17
  • Yes, in C you are only allowed to use an expression of type T to read member of that member is a variable declared as type T, or malloc'd space that was last written as type T. There are exceptions for closely related types such as signed/unsigned or const/nonconst, and T as character type. Look up "strict aliasing" for more info – M.M Apr 06 '16 at 21:18

1 Answers1

3

The original code returns from each case, the new code falls through... did you intend to fall through on the switch ie not use break?

Harry
  • 11,298
  • 1
  • 29
  • 43