From fa6b29f87b5014334d604012fb4a9201e6227c5b Mon Sep 17 00:00:00 2001 From: Sam Moore Date: Fri, 13 Sep 2013 01:04:48 +0800 Subject: [PATCH] Fix errors in refactored code NOTE to Jeremy: I was using 2 FILE* for read/writing But that seems to cause irregular behaviour (fread fails a lot). So I just made both the FILE* the same and put mutexes around all of Data_Save and Data_Read for now. --- server/data.c | 95 ++++++++++++++++++++++++++++------------------ server/data.h | 2 +- server/sensor.c | 46 +++++++++++++++++----- server/valgrind.sh | 2 +- 4 files changed, 97 insertions(+), 48 deletions(-) diff --git a/server/data.c b/server/data.c index 57b51ff..82aadb6 100644 --- a/server/data.c +++ b/server/data.c @@ -29,25 +29,30 @@ void Data_Open(DataFile * df, const char * filename) assert(df != NULL); // Set the filename - df->filename = filename; + df->filename = strdup(filename); // Set number of DataPoints df->num_points = 0; // Set write FILE* - df->write_file = fopen(filename, "w"); + df->write_file = fopen(filename, "w+"); if (df->write_file == NULL) { Fatal("Error opening DataFile %s - %s", filename, strerror(errno)); } // Set read FILE* - df->read_file = fopen(filename, "r"); + df->read_file = df->write_file; + + //NOTE: Opening the same file in read mode gives funny results; fread generally reads less than expected + // The strerror is: "Transport endpoint is not connected" + /* + fopen(filename, "r"); if (df->read_file == NULL) { Fatal("Error opening DataFile %s - %s", filename, strerror(errno)); } - + */ } @@ -65,7 +70,10 @@ void Data_Close(DataFile * df) df->read_file = NULL; df->write_file = NULL; + fclose(df->write_file); + // Clear the filename + free(df->filename); df->filename = NULL; } @@ -77,6 +85,7 @@ void Data_Close(DataFile * df) */ void Data_Save(DataFile * df, DataPoint * buffer, int amount) { + pthread_mutex_unlock(&(df->mutex)); assert(df != NULL); assert(buffer != NULL); assert(amount >= 0); @@ -97,8 +106,8 @@ void Data_Save(DataFile * df, DataPoint * buffer, int amount) } // Update number of DataPoints - pthread_mutex_lock(&(df->mutex)); - df->num_points += amount_written; + df->num_points += amount_written; + pthread_mutex_unlock(&(df->mutex)); } @@ -113,19 +122,21 @@ void Data_Save(DataFile * df, DataPoint * buffer, int amount) */ int Data_Read(DataFile * df, DataPoint * buffer, int index, int amount) { + pthread_mutex_lock(&(df->mutex)); + assert(df != NULL); assert(buffer != NULL); assert(index >= 0); assert(amount > 0); // If we would read past the end of the file, reduce the amount of points to read - pthread_mutex_lock(&(df->mutex)); - if (index + amount >= df->num_points) + + if (index + amount > df->num_points) { - Log(LOGDEBUG, "Requested %d points but will only read %d to get to EOF", amount, df->num_points - index); + Log(LOGDEBUG, "Requested %d points but will only read %d to get to EOF (%d)", amount, df->num_points - index, df->num_points); amount = df->num_points - index; } - pthread_mutex_unlock(&(df->mutex)); + // Go to position in file if (fseek(df->read_file, index*sizeof(DataPoint), SEEK_SET)) @@ -139,10 +150,11 @@ int Data_Read(DataFile * df, DataPoint * buffer, int index, int amount) // Check if correct number of points were read if (amount_read != amount) { - Fatal("Read %d points instead of %d from DataFile %s - %s", amount_read, amount, df->filename, strerror(errno)); + Log(LOGERR,"Read %d points instead of %d from DataFile %s - %s", amount_read, amount, df->filename, strerror(errno)); } - return amount; + pthread_mutex_unlock(&(df->mutex)); + return amount_read; } /** @@ -156,7 +168,8 @@ void Data_PrintByIndexes(DataFile * df, int start_index, int end_index, DataForm { assert(df != NULL); assert(start_index >= 0); - assert(end_index <= df->num_points-1); + assert(end_index >= 0); + assert(end_index <= df->num_points-1 || df->num_points == 0); const char * fmt_string; // Format for each data point char seperator; // Character used to seperate successive data points @@ -177,27 +190,32 @@ void Data_PrintByIndexes(DataFile * df, int start_index, int end_index, DataForm } DataPoint buffer[DATA_BUFSIZ]; // Buffer - - int index = start_index; - - // Repeat until all DataPoints are printed - while (index <= end_index) + // initialise buffer to stop stuff complaining + memset(buffer, 0, sizeof(DataPoint)*DATA_BUFSIZ); + + if (start_index < end_index) { - // Fill the buffer from the DataFile - int amount_read = Data_Read(df, buffer, index, DATA_BUFSIZ); - // Print all points in the buffer - for (int i = 0; i < amount_read; ++i) + int index = start_index; + // Repeat until all DataPoints are printed + while (index <= end_index) { - // Print individual DataPoint - FCGI_PrintRaw(fmt_string, buffer[i].time_stamp, buffer[i].value); - - // Last seperator is not required - if (index+1 < end_index) - FCGI_PrintRaw("%c", seperator); - - // Advance the position in the DataFile - ++index; + // Fill the buffer from the DataFile + int amount_read = Data_Read(df, buffer, index, DATA_BUFSIZ); + + // Print all points in the buffer + for (int i = 0; i < amount_read && index <= end_index; ++i) + { + // Print individual DataPoint + FCGI_PrintRaw(fmt_string, buffer[i].time_stamp, buffer[i].value); + + // Last seperator is not required + if (index+1 <= end_index) + FCGI_PrintRaw("%c", seperator); + + // Advance the position in the DataFile + ++index; + } } } @@ -223,9 +241,9 @@ void Data_PrintByIndexes(DataFile * df, int start_index, int end_index, DataForm void Data_PrintByTimes(DataFile * df, double start_time, double end_time, DataFormat format) { assert(df != NULL); - assert(start_time > 0); - assert(end_time > 0); - assert(end_time > start_time); + assert(start_time >= 0); + assert(end_time >= 0); + assert(end_time >= start_time); DataPoint closest; @@ -233,10 +251,13 @@ void Data_PrintByTimes(DataFile * df, double start_time, double end_time, DataFo int start_index = Data_FindByTime(df, start_time, &closest); // Start time is greater than most recent time stamp - if (start_index >= df->num_points-1 && closest.time_stamp < start_time) + if (start_index >= df->num_points-1) { - Data_PrintByIndexes(df, 0, 0, format); // Will print "empty" dataset - return; + if (start_index == 0 || closest.time_stamp < start_time) + { + Data_PrintByIndexes(df, 0, 0, format); // Will print "empty" dataset + return; + } } // Get finishing index diff --git a/server/data.h b/server/data.h index 726af5c..e52dadc 100644 --- a/server/data.h +++ b/server/data.h @@ -37,7 +37,7 @@ typedef struct FILE * read_file; // used for reading FILE * write_file; // used for writing int num_points; // Number of DataPoints in the file - const char * filename; // Name of the file + char * filename; // Name of the file pthread_mutex_t mutex; // Mutex around num_points } DataFile; diff --git a/server/sensor.c b/server/sensor.c index 8914817..227ea42 100644 --- a/server/sensor.c +++ b/server/sensor.c @@ -38,13 +38,20 @@ void Sensor_Init() */ void Sensor_Start(Sensor * s, const char * experiment_name) { + // Set filename char filename[BUFSIZ]; if (sprintf(filename, "%s_%d", experiment_name, s->id) >= BUFSIZ) { Fatal("Experiment name \"%s\" too long (>%d)", experiment_name, BUFSIZ); } + + Log(LOGDEBUG, "Sensor %d with DataFile \"%s\"", s->id, filename); + // Open DataFile Data_Open(&(s->data_file), filename); - + + s->record_data = true; // Don't forget this! + + // Create the thread pthread_create(&(s->thread), NULL, Sensor_Loop, (void*)(s)); } @@ -54,11 +61,14 @@ void Sensor_Start(Sensor * s, const char * experiment_name) */ void Sensor_Stop(Sensor * s) { + // Stop if (s->record_data) { s->record_data = false; - pthread_join(s->thread, NULL); - Data_Close(&(s->data_file)); + pthread_join(s->thread, NULL); // Wait for thread to exit + Data_Close(&(s->data_file)); // Close DataFile + s->newest_data.time_stamp = 0; + s->newest_data.value = 0; } } @@ -127,6 +137,7 @@ bool Sensor_Read(Sensor * s, DataPoint * d) if (result) { s->newest_data.time_stamp = d->time_stamp; + s->newest_data.value = d->value; } return result; } @@ -168,18 +179,23 @@ void Sensor_CheckData(SensorId id, DataPoint * d) void * Sensor_Loop(void * arg) { Sensor * s = (Sensor*)(arg); + Log(LOGDEBUG, "Sensor %d starts", s->id); // Until the sensor is stopped, record data points while (s->record_data) { DataPoint d; + //Log(LOGDEBUG, "Sensor %d reads data [%f,%f]", s->id, d.time_stamp, d.value); if (Sensor_Read(s, &d)) // If new DataPoint is read: { + //Log(LOGDEBUG, "Sensor %d saves data [%f,%f]", s->id, d.time_stamp, d.value); Data_Save(&(s->data_file), &d, 1); // Record it } } // Needed to keep pthreads happy + + Log(LOGDEBUG, "Sensor %d finished", s->id); return NULL; } @@ -280,17 +296,23 @@ void Sensor_Handler(FCGIContext *context, char * params) } SensorParams; // Fill values appropriately - if (!FCGI_ParseRequest(context, params, values, sizeof(values))) + if (!FCGI_ParseRequest(context, params, values, sizeof(values)/sizeof(FCGIValue))) { // Error occured; FCGI_RejectJSON already called return; } + // Get Sensor + Sensor * s = NULL; + // Error checking on sensor id if (id < 0 || id >= NUMSENSORS) { Log(LOGERR, "Invalid id %d", id); - FCGI_RejectJSON(context, "Invalid id"); // Whoops, I do still need this! + } + else + { + s = g_sensors+id; } DataFormat format = JSON; @@ -306,14 +328,13 @@ void Sensor_Handler(FCGIContext *context, char * params) Log(LOGERR, "Unknown format type \"%s\"", fmt_str); } - // Get Sensor - Sensor * s = g_sensors+id; + // Begin response Sensor_BeginResponse(context, id, format); // If a time was specified - if (FCGI_RECEIVED(values[START_TIME].flags) || FCGI_RECEIVED(values[END_TIME].flags)) + if ((s != NULL) && (FCGI_RECEIVED(values[START_TIME].flags) || FCGI_RECEIVED(values[END_TIME].flags))) { // Wrap times relative to the current time if (start_time < 0) @@ -325,14 +346,21 @@ void Sensor_Handler(FCGIContext *context, char * params) Data_PrintByTimes(&(s->data_file), start_time, end_time, format); } - else // No time was specified; just return a recent set of points + else if (s != NULL) // No time was specified; just return a recent set of points { pthread_mutex_lock(&(s->data_file.mutex)); int start_index = s->data_file.num_points-DATA_BUFSIZ; int end_index = s->data_file.num_points-1; pthread_mutex_unlock(&(s->data_file.mutex)); + // Bounds check + if (start_index < 0) + start_index = 0; + if (end_index < 0) + end_index = 0; + // Print points by indexes + Log(LOGDEBUG, "Sensor %d file \"%s\" indexes %d->%d", s->id, s->data_file.filename, start_index, end_index); Data_PrintByIndexes(&(s->data_file), start_index, end_index, format); } diff --git a/server/valgrind.sh b/server/valgrind.sh index 2a50865..3262b72 100755 --- a/server/valgrind.sh +++ b/server/valgrind.sh @@ -1,2 +1,2 @@ #!/bin/bash -valgrind --leak-check=full ./server +valgrind --leak-check=full --track-origins=yes ./server -- 2.20.1