I have a seg fault error in this piece of code, however I do not understand why. Can someone please explain what I'm doing wrong. I have commented the list of things I need to do in this function. I thought I was doing it right however when I print it out it turns out something totally different is actually happening.
void analyze_file(FILE *file, struct climate_info **states, int num_states) {
const int line_sz = 100;
char line[line_sz];
int currentStates = countStates(states);
while (fgets(line, line_sz, file) != NULL)
{
char* foundCode = strtok(line, "\t");
int rankOfState = compareOrder(states, foundCode, currentStates);
if(rankOfState == -1)
{
states[currentStates] = (struct climate_info *) malloc(sizeof(struct climate_info) *num_states);
strcpy((states[currentStates]) -> code, foundCode);
states[currentStates] -> num_records=1;
char* currentTimeStamp = strtok(NULL, "\t");
unsigned long TIMESTAMP;
sscanf(currentTimeStamp,"%lu", &TIMESTAMP);
char* currentGeol = strtok(NULL, "\t");
long long GEOL;
sscanf(currentGeol,"%llu", &GEOL);
char* currentHumidity = strtok(NULL, "\t");
double HUMIDITY;
sscanf(currentHumidity, "%lf",&HUMIDITY);
char* currentSnow = strtok(NULL, "\t");
float SNOW;
sscanf(currentSnow, "%f", &SNOW);
char* currentCloud = strtok(NULL, "\t");
double CLOUD;
sscanf(currentCloud, "%lf",&CLOUD);
char* currentLightning = strtok(NULL, "\t");
float LIGHTNING;
sscanf(currentLightning, "%f", &LIGHTNING);
char* currentPressure = strtok(NULL,"\t");
double PRESSURE;
sscanf(currentPressure, "%lf", &PRESSURE);
char* currentTemp = strtok(NULL, "\t\n");
double TEMP;
sscanf(currentTemp, "%lf",&TEMP);
if (TEMP < states[currentStates]->lo_temp_reading || states[currentStates]->lo_temp_timestamp == 0)
{
states[currentStates]->lo_temp_reading = TEMP;
states[currentStates]->lo_temp_timestamp = TIMESTAMP;
}
if (TEMP > states[currentStates]->hi_temp_reading || states[currentStates]->hi_temp_timestamp == 0)
{
states[currentStates]->hi_temp_reading = TEMP;
states[currentStates]->hi_temp_timestamp = TIMESTAMP;
}
currentStates++;
}
else
{
(*(states +rankOfState))->num_records +=1;
char* currentTimeStamp = strtok(NULL, "\t");
unsigned long TIMESTAMP;
sscanf(currentTimeStamp,"%lu", &TIMESTAMP);
char* currentGeol = strtok(NULL, "\t");
(*(states +rankOfState))->hi_millitime += *currentGeol;
char* currentHumidity = strtok(NULL, "\t");
double HUMIDITY;
sscanf(currentHumidity, "%lf",&HUMIDITY);
(*(states +rankOfState))->humidity += HUMIDITY;
char* currentSnow = strtok(NULL, "\t");
float SNOW;
sscanf(currentSnow, "%f", &SNOW);
(*(states +rankOfState))->snow += SNOW;
char* currentCloud = strtok(NULL, "\t");
double CLOUD;
sscanf(currentCloud, "%lf",&CLOUD);
(*(states +rankOfState))->cloud += CLOUD;
char* currentLightning = strtok(NULL, "\t");
float LIGHTNING;
sscanf(currentLightning, "%f", &LIGHTNING);
(*(states +rankOfState))->lightning += LIGHTNING;
char* currentPressure = strtok(NULL,"\t");
double PRESSURE;
sscanf(currentPressure, "%lf", &PRESSURE);
(*(states +rankOfState))->pressure += PRESSURE;
char* currentTemp = strtok(NULL, "\t\n");
double TEMP;
sscanf(currentTemp, "%lf",&TEMP);
(*(states +rankOfState))->temperature += TEMP;
if (TEMP <= states[currentStates]->lo_temp_reading)
{
states[currentStates]->lo_temp_reading = TEMP;
states[currentStates]->lo_temp_timestamp = *currentTimeStamp;
}
else if (*currentTemp > states[currentStates]->hi_temp_reading)
{
states[currentStates]->hi_temp_reading = *currentTemp;
states[currentStates]->hi_temp_timestamp = *currentTimeStamp;
}
currentStates++;
}
}
}
And if I comment out these lines, my output prints however, it only analyzes one line.
if (TEMP <= states[currentStates]->lo_temp_reading)
{
states[currentStates]->lo_temp_reading = TEMP;
states[currentStates]->lo_temp_timestamp = *currentTimeStamp;
}
else if (*currentTemp > states[currentStates]->hi_temp_reading)
{
states[currentStates]->hi_temp_reading = *currentTemp;
states[currentStates]->hi_temp_timestamp = *currentTimeStamp;
}
Here is are the helper functions I used:
int compareOrder(struct climate_info **states, char codex[3], int currentStates) //returns the order of each state in the array
{
int order = 0;
while (order < currentStates) //while order is less than number of states analyzed
{
if(strcmp((states[order])->code, codex) == 0) //if the states is present
{
return order;
}
order++; //increment here to check every line for when to update state codes
}
return -1; //returns -1 the state is not prsent in struct
}
int countStates(struct climate_info **states) //function to count number of states present
{
int num = 0;
while(num < 50 && states[num] != NULL)
{
num++;
}
return num;
}
There are quite a few issues with your code, unfortunately, there's quite a lot of information missing, so this answer is based on assumptions.
Edit: OK, this part got obsolete with your latest edit of the question; still: the signature proposed is supperior as it does not rely on the hard coded maximum array length, which you additionally provided as 'magic number'...
At first, countStates function; I assume it looks more or less as follows:
size_t countStates(struct climate_info** states)
{
struct climate_info** end = states;
while(*end)
++end;
return end - states;
}
Problem now is that you could easily iterate beyond the array bounds, causing undefined behaviour and possily crash already in this function. To fix, total array size/length as well:
size_t countStates(struct climate_info** states, size_t length)
{
struct climate_info** end = states;
while(length-- && *end)
++end;
return end - states;
}
Then let's get to the actual function (don't wonder about changed formatting/syntax, it is equivalent if not denoted otherwise – these are my personal preferences...):
// (size_t is more appropriate than int here...)
void analyze_file(FILE* file, struct climate_info* states[], size_t num_states)
{
const size_t line_sz = 100;
char line[line_sz];
size_t currentStates = countStates(states, num_states);
while (fgets(line, line_sz, file) != NULL)
{
char* foundCode = strtok(line, "\t");
int rankOfState = compareOrder(states, foundCode, currentStates);
if(rankOfState == -1)
{
// new states available at all?
if(currentStates == num_states)
{
// some appropriate error handling - need to decide you!
// for now, just returning from function:
return;
}
// just allocate one struct (assumption):
states[currentStates] = (struct climate_info *) malloc(sizeof(struct climate_info));
// always check the result of malloc!
if(!states[currentStates])
{
// allocation failed, no memory available on OS!!!
// some appropriate error handling - need to decide you!
// for now, just returning from function:
return;
}
// prefer strncpy to assure you don't copy past the end!
// sure, it will fill overdue bytes with 0, but still we are safer
// (assumption: climate_info contains an array!)
strncpy(states[currentStates]->code, foundCode, sizeof(states[currentStates]->code));
states[currentStates]->num_records = 1;
// ...
// now you created a new struct with malloc; be aware
// that memory is uninitialized and could contain *ANY*
// data, reading uninitialized memory is undefined behaviour!
//if (TEMP < states[currentStates]->lo_temp_reading || states[currentStates]->lo_temp_timestamp == 0)
//{
states[currentStates]->lo_temp_reading = TEMP;
// states[currentStates]->lo_temp_timestamp = TIMESTAMP;
//}
//if (TEMP > states[currentStates]->hi_temp_reading || states[currentStates]->hi_temp_timestamp == 0)
//{
states[currentStates]->hi_temp_reading = TEMP;
states[currentStates]->hi_temp_timestamp = TIMESTAMP;
//}
// (it's a new set anyway, so just set the values...)
currentStates++;
}
else
{
++states[rankOfState]->num_records; // just a little bit simpler...
// ...
states[rankOfState]->temperature += TEMP;
if (TEMP <= states[currentStates]->lo_temp_reading)
{
states[currentStates]->lo_temp_reading = TEMP;
states[currentStates]->lo_temp_timestamp = *currentTimeStamp;
}
// this should not have resulted in crash, but assign a bad value!
else if (TEMP /* *currentTemp */ > states[currentStates]->hi_temp_reading)
{
states[currentStates]->hi_temp_reading = TEMP /* *currentTemp */;
states[currentStates]->hi_temp_timestamp = *currentTimeStamp;
}
// same for timestamp in both branches: you don't want to assign first character of string,
// but the parsed value (e. g. 1012, if string was "1012", *... would deliver 49
// (ASCII code of character `1`; assuming you have ASCII compatible encoding)
// wrong in this branch: you did NOT add a new element
//currentStates++;
}
}
}
As indicated already: This is based on what I think you are trying to do. Leave a comment if my assumptions are wrong...