1

edit for clarity:

fscanf is working the way it is written here...it's getting the data into the array that's broken.

another edit for clarity:

The only point of failure is the memcpy line on the second iteration through the loop where fileindex is 0 and i is 1. the memcpy line works fine when i is 0.


I'm converting a program that used to read data from a binary file directly into an array using fread into reading from ascii text. Here's the setup:

#define MAX_FLIGHT_ENTRIES    27000
#define MAX_PLANES_IN_A_FLIGHT 10

typedef struct {
    double local_x;
    double local_y;
    double local_z;
    float pitch;
    float roll;
    float heading;
    float gearpos;
    float flappos;
    float speedbrakepos;
    float canopypos;
    float afterburnerOn;
    float kias;
    float time;     // record timestamp
} FLIGHT_ENTRY_TYPE;
static FLIGHT_ENTRY_TYPE FlightEntries [MAX_PLANES_IN_A_FLIGHT][MAX_FLIGHT_ENTRIES];

The way this used to work is, in a loop, the array would be filled via:

fread (&FlightEntries[fileIndex][i], sizeof (FLIGHT_ENTRY_TYPE), 1, pFile);

And I believe this would actually instantiate each entry in the array by putting the data directly into memory. Now I'm reading from a text file and I've tried everything I can to get the values into the array, but it only ever writes the first entry FlightEntries[0][0]. Any attempt to read or write to FlightEntries[0][1] crashes. Here's my current best attempt.

for ( i = 0; i < MAX_FLIGHT_ENTRIES; i++)
{               
        // If the file end is found before it should be, set values to defaults
        // and save the file
        if (feof(pFile))
        {
            FlightInfo[fileIndex].endFrameIndex = i - 1;
            break;
        }
        else
        {
            float plocalx, plocaly, plocalz;
            float ppitch, proll, pheading, pgearpos, pflappos, pbrakepos, pcanopypos, pafterburnon, pkias, ptime;
            
            int fresult;
            fresult = fscanf(pFile, "%f %f %f %f %f %f %f %f %f %f %f %f %f\n",
                &plocalx,
                &plocaly,
                &plocalz,
                &ppitch,
                &proll,
                &pheading,
                &pgearpos,
                &pflappos,
                &pbrakepos,
                &pcanopypos,
                &pafterburnon,
                &pkias,
                &ptime);

                FLIGHT_ENTRY_TYPE newEntry;

            newEntry.local_x = (double)plocalx;
            newEntry.local_y = (double)plocaly;
            newEntry.local_z = (double)plocalz;
            newEntry.pitch = ppitch;
            newEntry.roll = proll;
            newEntry.heading = pheading;
            newEntry.gearpos = pgearpos;
            newEntry.flappos = pflappos;
            newEntry.speedbrakepos = pbrakepos;
            newEntry.canopypos = pcanopypos;
            newEntry.afterburnerOn = pafterburnon;
            newEntry.kias = pkias;
            newEntry.time = ptime;

            memcpy (&FlightEntries[fileIndex][i], &newEntry, sizeof FLIGHT_ENTRY_TYPE);
        }
}

I don't think the array entries are getting allocated properly. I've tried accessing the individual structure members directly via FlightEntries[fileIndex][i].local_x = (double)plocalx; and I've also tried using memcpy to do the same thing for each member...am I doing my pointers wrong or something? I have no idea where to go with this. Every time I get past one stumbling block, something else comes up and I think it's all related to the array as opposed to reading from file. Do I have to do something to allocate space?

The big question is:

What does fread do with binary data in FlightEntries[0][1] that memcpy isn't doing? And is my memcpy line correct? Do I need to do some kind of malloc?

Community
  • 1
  • 1
Robbie P.
  • 151
  • 1
  • 11
  • 1
    instead of filling a temporary struct instance, why don't you read into the FlightEntries directly since you don't seem to need any validation? – sithereal Mar 04 '15 at 14:55
  • 3
    `fresult = fscanf(` I dont see the fresult varable checked or used antwhere. Also: `feof()` is wrong. almost. always. – wildplasser Mar 04 '15 at 14:57
  • @sithereal in the last paragraph after the code, I mention that I tried that...unless there's a different method you're thinking of? The original problem was fscanf not reading the double properly...that's why I read into local variables and cast the three doubles. Got an example? I think it's a memory issue with the array. – Robbie P. Mar 04 '15 at 14:59
  • @wildplasser I have a block of code that logs fresult so I can verify that it's reading the correct number of entries. As long as I don't attempt to put anythign into the array, it reads the whole file correctly and I can log the whole file via writing the newEntry elements to the log, so the data is coming in properly, I'm just unable to access any additional entries of the FlightEntries array after [0][0]. – Robbie P. Mar 04 '15 at 15:01
  • did you try using %lf instead of %f for reading doubles? – sithereal Mar 04 '15 at 15:01
  • @sithereal yes, and for some reason it didn't work...but what I have currently definitely reads and casts the values correctly...I just can't get them into the FlightEntries array no matter what I do. – Robbie P. Mar 04 '15 at 15:03
  • how is `FlightEntries` defined (and maybe allocated)? -- forget about that, I was blind -- Are you sure that `fileIndex` is correct? – Ingo Leonhardt Mar 04 '15 at 15:07
  • @IngoLeonhardt FlightEntries is defined in the first block of code. And my omitted log function writes fileIndex as 0, and i as 0...but it never makes it to fileindex [0] i [1] because it crashes when I try to access it...first time through this loop works flawlessly. – Robbie P. Mar 04 '15 at 15:09
  • 1
    It's generally a bad idea to do direct I/O of full structures, since there can be padding inside, for alignment reasons. This padding can change with various compilers, options, or platforms. So it's a very brittle format. – unwind Mar 04 '15 at 15:11
  • @unwind the data file is written by the same program...Also it's a very temporary issue I need to get through for a short term result...It used to be binary, but I need to be able to edit the files for a single instance, then it will go back to being binary and I'll never have to deal with this again. I just need to be able to get the data into the array. – Robbie P. Mar 04 '15 at 15:13
  • 1
    @RobbieP. Right. I was actually confused. You're using text, which is *good*, my entire point was that *binary* is bad. :) – unwind Mar 04 '15 at 15:18
  • @unwind gotcha. Well, the reading of the text is working, for the whole file (~1520 entries in my example). I'm unable to duplicate whatever fread does to add the data to the array. – Robbie P. Mar 04 '15 at 15:21
  • The problem is that the answer to your question "what does fread do with binary data in FlightEntries[0][1] that memcpy isn't doing?" simply is --- Nothing. I think, the problem must be somewhere else in your code. I think you should try using tools like gdb or valgrind to get a clue – Ingo Leonhardt Mar 04 '15 at 15:37
  • @IngoLeonhardt debugging is difficult for me because I'm using freetype, and for some reason it has a memory leak that prevents me from running a debug build, and since it's not my library, I'm stuck with the memory leak. – Robbie P. Mar 04 '15 at 15:42
  • You could experience a page access violation. Try allocating the array explicitly on the heap using `calloc` or similar and try again... – king_nak Mar 04 '15 at 15:48
  • @king_nak this sounds promising...never done this before...where would this be done? in the loop? am I allocating for each entry as it's read in? could you post an example as an answer? – Robbie P. Mar 04 '15 at 15:54
  • The problem I described could only occur if you are not using the same global static `FlightEntries` array as in your original code, but a locally created array in your function. Is this the case? – king_nak Mar 04 '15 at 16:15
  • @king_nak I am trying to add a locally defined single struct (`newEntry`) with values into the global static `FlightEntries` array. So I think the answer is no...the only attempt to write to `FlightEntries` is the global static one. – Robbie P. Mar 04 '15 at 16:27
  • Can you tell me what ***MAX_FLIGHT_ENTRIES*** represents? - My guess is that your 2 dimensional array of structs should only be a one dimensional array of structs. This would simplify your memcpy() issue – ryyker Mar 04 '15 at 17:36

2 Answers2

3

...used to read data from a binary file directly into an array using fread....
EDIT
Now I'm reading from a text file and I've tried everything I can to get the values into the array.

Unless the contents of your file match the current definition of the FLIGHT_ENTRY_TYPE struct, and perhaps these definitions:

#define MAX_FLIGHT_ENTRIES    27000
#define MAX_PLANES_IN_A_FLIGHT 10

There will be problems with your read attempt, no matter what you try. (The file contents, and the struct definition MUST be in alignment to do what you are trying to do. Because you are using a text file, this should be easily verifiable.)

Also, feof(pFile) is rarely a good choice for reading a file

Consider changing it to something like this:(pseudo code)

FLIGHT_ENTRY_TYPE newEntry;
int len = sizeof(FLIGHT_ENTRY_TYPE)*2;// *2 to account for comma delimiters, etc.  Change as needed
char **stringArray = {0};
//see edits in answer below for defintion of allocArray()
stringArray = allocMemory(stringArray, MAX_PLANES_IN_A_FLIGHT, len);        
FILE *fp = fopen(".\\filename.bin", "rb");
if(fp && stringArray)
{   
    while(fgets(stringArray[i], len, fp)
    {
         fresult = sscanf(stringArray[i], "%f %f %f %f %f %f %f %f %f %f %f %f %f\n",
                    &plocalx,
                    &plocaly,
                    &plocalz,
                    &ppitch,
                    &proll,
                    &pheading,
                    &pgearpos,
                    &pflappos,
                    &pbrakepos,
                    &pcanopypos,
                    &pafterburnon,
                    &pkias,
                    &ptime);  
          if(fresult > 0)
          { ...assign values to newEntry struct...  }

    }
    fclose(fp);
    freeMemory(stringArray,MAX_PLANES_IN_A_FLIGHT);   
}

The first three members of your struct are double values, and should therefore be read into double values. If you decide to make that change:

float plocalx, plocaly, plocalz; 

TO:

double plocalx, plocaly, plocalz;

Be sure to also Change:

fresult = fscanf(pFile, "%f %f %f %f %f %f %f %f %f %f %f %f %f\n", 

To:

fresult = fscanf(pFile, "%lf %lf %lf %f %f %f %f %f %f %f %f %f %f\n",  

You should also check the return value of sscanf()
The return value: Converts input from the specified source string into a series of values according to the specifiers in formatString. If an input failure occurs before the first conversion, the function returns EOF (-1); otherwise, the function returns the number of input items assigned. If copying takes place between objects that overlap, the behavior is undefined.

EDIT 2
If you do need to create an array of strings (for each line of the file), these functions will help:

char ** allocMemory(char ** a, int numStrings, int maxStrLen)
{
    int i;
    a = calloc(sizeof(char*)*(numStrings+1), sizeof(char*));
    for(i=0;i<numStrings; i++)
    {
      a[i] = calloc(sizeof(char)*maxStrLen + 1, sizeof(char));
    }
    return a;
}

void freeMemory(char ** a, int numStrings)
{
    int i;
    for(i=0;i<numStrings; i++)
        if(a[i]) free(a[i]);
    free(a);
}

//usage:
#define MAX_FLIGHT_ENTRIES    27000
#define MAX_PLANES_IN_A_FLIGHT 10
int main(void)
{
    char **stringArray = {0};
    stringArray = allocMemory(stringArray, MAX_PLANES_IN_A_FLIGHT, sizeof FLIGHT_ENTRY_TYPE);
    ...//read data into string array as shown in code above
    return 0;
}   

The concept here is that each line of the file is assumed to represent the data to populate one struct instance of the FLIGHT_ENTRY_TYPE struct. Stated differently, one line in the file is read for each struct in the array of structs.

Community
  • 1
  • 1
ryyker
  • 22,849
  • 3
  • 43
  • 87
  • fscanf is working perfectly...as long as I don't attempt to access the FlightEntries array, I can read in and access the whole file without error. I've even gone so far as to use the newEntry that's created to re-dump the values to another file to verify that it's getting read properly. Even if I change to %lf, it'll still crash when I attempt to write to the array. :( – Robbie P. Mar 04 '15 at 15:05
  • 1
    but `plocal[xyz]` are `float` not `double`, so `"%f"` is correct – Ingo Leonhardt Mar 04 '15 at 15:05
  • @RobbieP. - Edited to identify a couple of other things to look at. Yes, your formatting of floats was correct, but some other issues may be contributing to the problem. Are you sure the file definition matches the #defines you are using to read it in? – ryyker Mar 04 '15 at 15:44
  • @ryyker ick...problem is you're workign in the wrong direction...it used to be binary, I converted it to text, not the other way around. The file I'm reading is an ascii text file and the fscanf is working the way it's supposed to. It's putting the data into the array that is the problem, not getting it from the file. – Robbie P. Mar 04 '15 at 15:49
  • @Robbie - well I will have to edit my suggestion then :), but it will still include checking the results of your `fscanf()`, and using `fopen()` and then 'fgets()` (instead of `fread()`) – ryyker Mar 04 '15 at 15:53
  • @ryyker thanks! Another user is suggesting calloc...any thoughts? – Robbie P. Mar 04 '15 at 15:57
  • @ryyker I've marked your answer as useful, because it helps me more appropriately get the data from the file...but it's still not helping me get the newEntry into the FlightEntries array. – Robbie P. Mar 04 '15 at 16:08
  • @RobbieP. - It depends - I still do not know if the file you are reading in is described perfectly by `#define MAX_FLIGHT_ENTRIES 27000` and `#define MAX_PLANES_IN_A_FLIGHT 10`. If no, then yes, you need to read the file, determine number of lines, length of each line, and then allocate memory accordingly. You would need to create a 2 dim array. See edit. – ryyker Mar 04 '15 at 16:54
  • 1
    "If an input failure occurs before any conversion, the function returns EOF (-1);" is better re-stated as "If an input failure occurs before _the first_ conversion, the function returns EOF (-1);" – chux - Reinstate Monica Mar 04 '15 at 22:48
  • @chux - Thanks! I took that right out of the documentation for my system, but you are correct. And I made an edit to reflect that. – ryyker Mar 04 '15 at 23:05
0

Extenuating unlisted circumstances.

Turns out, since I couldn't debug, I had placed a log file snippet at the end of the loop iteration. I believe that instantiating the logfile was corrupting the array and preventing the array from being written to. Possibly a heap issue, but I'm not sure. I've learned quite a bit from this exercise about dealign with memory pointers as well as better ways to read and write data from file. So Thank you all for the help.

Robbie P.
  • 151
  • 1
  • 11
  • Be sure to mark your answer as _anwered_ once you are able to. :) (hollow green check mark) – ryyker Mar 04 '15 at 23:47