0
votes

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.

1
You should make your struct packed. see stackoverflow.com/questions/1537964/…koder
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.Ian Abbott
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
"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
I added #pragma pack(push, 1) to the header file but I am still not getting the correct data...Rugbrød

1 Answers

2
votes

/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.