fread() not reading struct data correctly after upgrading C++ tools version

Question

I have an older project that reads a binary file into a struct using fread(). It uses Visual Studio 2017 (v141)

I upgraded the project to the newest C++ tools version (v142) for VS 2019, and also changed the solution from AnyCPU to x86.

I also changed Struct Member alignment from: 1 Byte (/Zp1) To: Default

because of this error:

Error C2338 Windows headers require the default packing option. Changing this can lead to memory corruption. This diagnostic can be disabled by building with WINDOWS_IGNORE_PACKING_MISMATCH defined.

But now the data is not being read correctly anymore.

This is the code I use to read the binary files:

FILE* RecipeFile;
    short ReturnValue = 0; //AOK

    if ((RecipeFile = fopen(rcpFile, "rb")) == NULL) {
        ReturnValue = -1; //File open error
        return(ReturnValue);
    }

    long sizeOfItem;

    // obtain file size:
    fseek(RecipeFile, 0, SEEK_END);
    sizeOfItem = ftell(RecipeFile); // gives 771088
    rewind(RecipeFile);

    //sizeOfItem = sizeof(recipe); // gives 824304??

    // now read the file contents:
    int noOfItemsRead = fread(&recipe, sizeOfItem, 1, RecipeFile);

recipe is a struct. See below.

I have noticed that sizeof(recipe) gives a different result than the file size. With toolset 141, I get 798276 bytes. With toolset 142, I get 824304 bytes. The actual file size is 771088 bytes.

Is the issue really caused by the change in struct member alignment?

How can I fix the error so the files are being read correctly again?

EDIT: I tried adding pragma pack directives to the struct like so:

#pragma pack(push, 1)
struct tRecipe {   
    RMPTableDescriptorType O2Flow;
    RMPTableDescriptorType HeFlow;
    RMPTableDescriptorType SiCl4Flow;
    RMPTableDescriptorType GeCl4Flow;
    RMPTableDescriptorType Extra_Flow;
    RMPTableDescriptorType POCl3Flow;
    RMPTableDescriptorType C2F6Flow;
    RMPTableDescriptorType SiF4Flow;
    RMPTableDescriptorType Cl2Flow;
    RMPTableDescriptorType BCl3Flow;
    RMPTableDescriptorType TTC_Temp;
    RMPTableDescriptorType TTC_H2Flow;
    RMPTableDescriptorType TTC_Ratio;
    RMPTableDescriptorType LCC_Speed;
    RMPTableDescriptorType LSC_Speed;
    RMPTableDescriptorType LTC_Speed;
    RMPTableDescriptorType TDS_Cursor;
    RMPTableDescriptorType TDC_Ctrl;
    RMPTableDescriptorType TPC_Ctrl;
    RMPTableDescriptorType TSS_Cursor;
    DLUTableDescriptorType DLU;
    EXHTableDescriptorType EXH;
    GENTableDescriptorType GEN;
    PARTableDescriptorType PAR;
    REFTableDescriptorType REF;
    SETTableDescriptorType SET;
    SUPTableDescriptorType SUP;
    TDCTableDescriptorType TDC;
    TDSTableDescriptorType TDS;
    TSSTableDescriptorType TSS;
    TTCTableDescriptorType TTC;
    TPCTableDescriptorType TPC;
};
#pragma pack(pop)

typedef struct
{
    RParam                      Value;
    UInt16                      Duration;
    OptType                     Opt;
#if DOS
    int                         Unused16BitVar; /* Established to allow Win32 NT Code to use 32bit */
#endif
}
RMPElementDescriptorType;

/*-----------------------------------------------------------------------------*/
typedef struct
{
    UInt16                      StartNoOf;
    UInt16                      EndNoOf;
    Char8                       StartRampType;
    Char8                       EndRampType;
    RMPElementDescriptorType    RMPElementArray[RAMP_ELEM_NO_OF];
}
RMPRampDescriptorType;

/*-----------------------------------------------------------------------------*/
typedef struct
{
    UInt16                 SizeInfo;
    Char8                  DBPTxtInfo[DBP_TXT_NO_OF];

    RMPRampDescriptorType  RMPRampArray[RAMP_NO_OF];
}
RMPTableDescriptorType;

but it does not seem to have any effect on the error.

You should make your struct packed. see https://stackoverflow.com/questions/1537964/visual-c-equivalent-of-gccs-attribute-packed — koder, Mar 08 '21 at 12:46
The MSVC compiler uses the `pack` pragma to change packing on the fly. So you could surround the definition of `struct recipe` with `#pragma pack(push, 1)` before the definition and `#pragma pack(pop)` after the definition. That would make `struct recipe` packed with 1-byte alignment, but everything else packed with default alignment. See [`pack` pragma](https://learn.microsoft.com/en-us/cpp/preprocessor/pack?view=msvc-160). — Ian Abbott, Mar 08 '21 at 12:47
Your file is an "interface" to anyone who wants to read or write it. It requires that the packing is clearly defined so the reading and writing can be done compiler-independent by anyone. — Paul Ogilvie, Mar 08 '21 at 13:35
_"recipe is a struct which I won't include here because it is very big."_ you should at least provide an elided version - or even just its name/tag - the answer will make more sense if it uses your naming. — Clifford, Mar 08 '21 at 13:58
I added #pragma pack(push, 1) to the header file but I am still not getting the correct data... — Rye bread, Mar 08 '21 at 13:59
What @PaulOgilvie said, but there may still be byte-order incompatibilities if you build your code for a platform with different endianness and your struct includes integer of fp members. — Clifford, Mar 08 '21 at 14:04
@Clifford, you are correct. Byte ordering and character set are also part of the interface specification (design). — Paul Ogilvie, Mar 08 '21 at 14:06
As you can see now, writing binary data structures to a file is a bad choice.If you are not able to find compiler settings to reproduce the original layout you might have to find out the original layout, read the data in a buffer and to copy the data to a structure field-by-field in a portable way. When you add `#pragma pack(push, 1)`, is the structure size (`sizeof(recipe)`) different from the result without it? Please also show (at least some of) the type definitions for `RMPTableDescriptorType` etc. — Bodo, Mar 08 '21 at 14:17

Clifford · Answer 1 · 2021-03-08T14:21:04.827

2

/Zp1 was always a bad idea - it applies packing globally. By changing the packing, you render existing files incompatible. Instead you should pack selectively:

#pragma pack(1) // 1 byte packing
struct sRecipe
{
   ...
} ;
#pragma pack() // restore default packing

However that may not resolve all compatibility issues across compilers, compiler versions and targets (and you also changed the target from AnyCPU to x86). In practice is is better to serialise/deserialise data saved to a file using CSV, XML, or binary with "network byte order" (using htonl()/nltoh() et al) for example, to avoid alignment and byte-order issues of raw structures.

Essentially you should write code to explicitly generate the format you have specified for the file, byte-by-byte for binary files or using unambiguous string representations otherwise.

edited Mar 08 '21 at 14:21

answered Mar 08 '21 at 14:02

Clifford

88,407
13
85
165

I tried this in various forms, but it had no effect. The only thing that worked was switching back to Struct Member Alignment: 1 Byte (/Zp1) – Rye bread Mar 09 '21 at 12:21
@Rugbrød The packing directive was the quick-and-dirty solution. I also suggested you deserialused. I am guessing that is not what you tried. You would do well perhaps to understand _why_ it does not work, and for that you should use the debugger to view the memory associated with the read and why it does not fit the structure. – Clifford Mar 09 '21 at 13:15

fread() not reading struct data correctly after upgrading C++ tools version

1 Answers1