From 6125043513ed046e45a06bb580f5214c96a95039 Mon Sep 17 00:00:00 2001 From: Adrien Bourmault Date: Wed, 22 Sep 2021 16:43:01 +0200 Subject: [PATCH] Corrected memory and printing errors --- Makefile | 2 +- include/base.h | 2 +- include/terminal.h | 8 ++++---- src/cli.c | 4 ++-- src/cmds.c | 28 ++++++++++++++++++++++++++-- src/main.c | 8 ++++---- src/model.c | 19 ++++++++++++++----- src/parsing.c | 26 ++++++++++++++------------ src/scheduler.c | 7 +++++-- src/server.c | 18 +++++++++--------- 10 files changed, 80 insertions(+), 42 deletions(-) diff --git a/Makefile b/Makefile index a519794..0a1f301 100644 --- a/Makefile +++ b/Makefile @@ -122,7 +122,7 @@ run-gdb: all -ex "set args -C debian/etc -M debian/var/models -U debian/var/users" run-valgrind: all - @valgrind --leak-check=full -s \ + @valgrind --leak-check=full --show-leak-kinds=all -s \ bin/gem-graph-server -C debian/etc -M debian/var/models \ -U debian/var/users diff --git a/include/base.h b/include/base.h index 0b82901..ed2dc5f 100644 --- a/include/base.h +++ b/include/base.h @@ -37,7 +37,7 @@ #define LOGMSG "[%s]" #define printLog(FORMAT, ...) printf("\e[0m" LOGMSG " " FORMAT "\e[0m", \ __func__, ##__VA_ARGS__) -#define printErr(FORMAT,...) sprintf(stderr, "\e[0m" LOGMSG " " FORMAT "\e[0m",\ +#define printErr(FORMAT,...) fprintf(stderr, "\e[0m" LOGMSG " " FORMAT "\e[0m",\ __func__, ##__VA_ARGS__) #define LEN(x) (sizeof(x) / sizeof((x)[0])) diff --git a/include/terminal.h b/include/terminal.h index 7c1f68d..1a5107e 100644 --- a/include/terminal.h +++ b/include/terminal.h @@ -74,7 +74,7 @@ static inline int TermGetch(bool nonBlocking) fcntl(0, F_SETFL, O_NONBLOCK); if(tcgetattr(0, &old) < 0) { - printLog("%sError getting terminal settings! (%s)\n", + printErr("%sError getting terminal settings! (%s)\n", C_COLOR_RED, strerror(errno)); return -1; @@ -84,7 +84,7 @@ static inline int TermGetch(bool nonBlocking) old.c_lflag &= ~ECHO; // set no echo mode if(tcsetattr(0, TCSANOW, &old) < 0) { - printLog("%sError setting terminal settings! (%s)\n", + printErr("%sError setting terminal settings! (%s)\n", C_COLOR_RED, strerror(errno)); return -1; @@ -96,7 +96,7 @@ static inline int TermGetch(bool nonBlocking) if(errno == EAGAIN) return 0; - printLog("%sError reading character! (%s)\n", + printErr("%sError reading character! (%s)\n", C_COLOR_RED, strerror(errno)); return -1; @@ -106,7 +106,7 @@ static inline int TermGetch(bool nonBlocking) old.c_lflag |= ECHO; // set echo mode if(tcsetattr(0, TCSADRAIN, &old) < 0) { - printLog("%sError resetting terminal settings! (%s)\n", + printErr("%sError resetting terminal settings! (%s)\n", C_COLOR_RED, strerror(errno)); return -1; diff --git a/src/cli.c b/src/cli.c index b298b28..1631b3d 100644 --- a/src/cli.c +++ b/src/cli.c @@ -444,7 +444,7 @@ int main(void) // Socket creation sockfd = socket(AF_INET, SOCK_STREAM, 0); if (sockfd == -1) { - printLog("%sSocket creation failed! (%s)\n", + printErr("%sSocket creation failed! (%s)\n", C_COLOR_RED, strerror(errno)); return 1; @@ -459,7 +459,7 @@ int main(void) // Connect to the server if (connect(sockfd, (struct sockaddr*)&servaddr, sizeof(servaddr)) != 0) { - printLog("%sConnection failed! (%s)\n", + printErr("%sConnection failed! (%s)\n", C_COLOR_RED, strerror(errno)); return 2; diff --git a/src/cmds.c b/src/cmds.c index e6557c2..9de3fd4 100644 --- a/src/cmds.c +++ b/src/cmds.c @@ -34,7 +34,8 @@ char *CmdModel(char *buf, char **argv, Server_t *args) // invalid use if (!argv[1]) { - strcat(buf, "{create | delete | load | run | stop | list | info}\n"); + strcat(buf, "{create | delete | load | unload | run | stop | list |" + "info}\n"); goto CmdModelEnd; } @@ -100,6 +101,28 @@ char *CmdModel(char *buf, char **argv, Server_t *args) goto CmdModelEnd; } + else if (strcmp(argv[1], "unload") == 0) { + if (!argv[2]) { + goto unloadEnd; + } + + id = (int) strtol(argv[2] + 3, NULL, 10); + + if (id == 0 || ModelUnload(id) < 0) { + printErr("Failed to unload model id %d\n", id); + sprintf(buf + strlen(buf), "Failed to unload model id %d\n", id); + goto CmdModelEnd; + } + + sprintf(buf + strlen(buf), "Model id %d unloaded\n", id); + goto CmdModelEnd; + unloadEnd: + // invalid use + strcat(buf, "Unloads a model structure\n"); + strcat(buf, "Usage: model unload id=ID\n"); + goto CmdModelEnd; + } + else if (strcmp(argv[1], "delete") == 0) { if (!argv[2]) { goto deleteEnd; @@ -172,7 +195,8 @@ char *CmdModel(char *buf, char **argv, Server_t *args) } // invalid use - else strcat(buf, "{create | delete | load | run | stop | list | info}\n"); + else strcat(buf, "{create | delete | load | unload | run | stop | list |" + "info}\n"); CmdModelEnd: return buf; diff --git a/src/main.c b/src/main.c index 6dd30b4..a7e9667 100644 --- a/src/main.c +++ b/src/main.c @@ -59,12 +59,12 @@ int main(int argc, char **argv) strcpy(parameters.modelDir, optarg); break; case 'U': - printLog("User : %s\n", optarg); + printLog("Users : %s\n", optarg); parameters.userDir = (char*) calloc(1, strlen(optarg) + 1); strcpy(parameters.userDir, optarg); break; case ':': - printLog("Option missing argument : %c\n", optopt); + printErr("Option missing argument : %c\n", optopt); if (parameters.configDir) free(parameters.configDir); @@ -77,7 +77,7 @@ int main(int argc, char **argv) return ENOSYS; break; case '?': - printLog("Unknown option : %c\n", optopt); + printErr("Unknown option : %c\n", optopt); if (parameters.configDir) free(parameters.configDir); @@ -93,7 +93,7 @@ int main(int argc, char **argv) } if (!parameters.configDir | !parameters.modelDir | !parameters.userDir) { - printLog("Missing arguments\n"); + printErr("Missing arguments\n"); if (parameters.configDir) free(parameters.configDir); diff --git a/src/model.c b/src/model.c index b214baa..21491e0 100644 --- a/src/model.c +++ b/src/model.c @@ -64,6 +64,7 @@ int ModelLoad(int id) printLog("Loading model id %d (/%d models)...\n", id, knownModelSize); + // Creating structure for the Scheduler knownModel[id-1]->scheduler = (Scheduler_t*) calloc(1, sizeof(Scheduler_t)); @@ -79,12 +80,20 @@ int ModelLoad(int id) int ModelUnload(int id) { // Destroy scheduler - SchedDestroy(loadedModel[id-1]->scheduler); - loadedModel[id-1]->scheduler = NULL; + if (id > loadedModelSize) + return -1; + if (loadedModel[id-1]->scheduler) { + SchedDestroy(loadedModel[id-1]->scheduler); + loadedModel[id-1]->scheduler = NULL; + } + + // Prevent fragmentation by moving data in the newly freed slot + if (id-1 < loadedModelSize) { memmove(&loadedModel[id-1], &loadedModel[id-1] + sizeof(Model_t*), loadedModelSize - (id-1)); + } // Decrement loaded model index loadedModelSize--; @@ -243,7 +252,7 @@ void ModelSystemInit(Parameters_t *parameters) // Open model directory if ((modelDir = opendir(parameters->modelDir)) <= 0) { - printLog("Could not open %s\n", parameters->modelDir); + printErr("Could not open %s\n", parameters->modelDir); ModelSystemDestroy(); kill(getpid(), SIGTERM); return; @@ -275,7 +284,7 @@ void ModelSystemInit(Parameters_t *parameters) // Ask to parse the new model if (ParseModelIdentityXML(newModel, parameters) != 0) { - printLog("Deleting invalid model %s from known list\n", + printErr("Deleting invalid model %s from known list\n", newModel->name); ModelDelete(newModel->id); continue; @@ -283,7 +292,7 @@ void ModelSystemInit(Parameters_t *parameters) // Check model is valid and/or parsed if (newModel->validated == false) { - printLog("Deleting invalid model %s from known list\n", + printErr("Deleting invalid model %s from known list\n", newModel->name); ModelDelete(newModel->id); continue; diff --git a/src/parsing.c b/src/parsing.c index c1336ef..edcecb5 100644 --- a/src/parsing.c +++ b/src/parsing.c @@ -336,7 +336,7 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) xmlDocument = xmlParseFile(model->filename); if (xmlDocument == NULL) { - printLog("Can't parse model file at '%s'.\n", model->filename); + printErr("Can't parse model file at '%s'.\n", model->filename); return -1; } @@ -344,7 +344,7 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) currentNode = xmlDocGetRootElement(xmlDocument); if (currentNode == NULL) { - printLog("Invalid model file at '%s', document empty !\n", + printErr("Invalid model file at '%s', document empty !\n", model->filename); xmlFreeDoc(xmlDocument); free(schemPath); @@ -353,7 +353,7 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) // Checking that XML file is actually a model if (xmlStrcmp(currentNode->name, (const xmlChar *) "gem-graph-model")) { - printLog("Invalid model file at '%s', " + printErr("Invalid model file at '%s', " "root node is not !\n", model->filename); xmlFreeDoc(xmlDocument); @@ -365,12 +365,12 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) version = xmlGetProp(currentNode, (xmlChar*)"version"); // Check version is present if (!version) { - printLog("Missing version for model %s \n", model->name); + printErr("Missing version for model %s \n", model->name); xmlFreeDoc(xmlDocument); free(schemPath); return -4; } else if (strlen((char*)version) > MODEL_STRING_SIZE) { - printLog("Invalid version number for model %s \n", model->name); + printErr("Invalid version number for model %s \n", model->name); free(version); xmlFreeDoc(xmlDocument); free(schemPath); @@ -384,7 +384,7 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) // Loading schema file schemFile = xmlSchemaNewParserCtxt(schemPath); if (schemFile == NULL) { - printLog("Invalid gem-graph version %s in model %s: no schema present", + printErr("Invalid gem-graph version %s in model %s: no schema present", version, model->name); free(version); xmlFreeDoc(xmlDocument); @@ -394,9 +394,9 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) // Loading schema file content schemPtr = xmlSchemaParse(schemFile); - xmlSchemaFreeParserCtxt(schemFile); if (schemPtr == NULL) { - printLog("Invalid schema file, version %s\n", version); + printErr("Invalid schema file, version %s\n", version); + xmlSchemaFreeParserCtxt(schemFile); free(version); xmlFreeDoc(xmlDocument); free(schemPath); @@ -406,11 +406,12 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) // Creating validating context schemValidator = xmlSchemaNewValidCtxt(schemPtr); if (schemValidator == NULL) { + xmlSchemaFreeParserCtxt(schemFile); xmlSchemaFree(schemPtr); free(version); xmlFreeDoc(xmlDocument); free(schemPath); - printLog("An error occured preparing schema file, version %s\n", version); + printErr("An error occured preparing schema file, version %s\n", version); return -7; } @@ -441,6 +442,7 @@ int ParseModelIdentityXML(Model_t *model, Parameters_t *params) currentNode = currentNode->next; } + xmlSchemaFreeParserCtxt(schemFile); xmlSchemaFreeValidCtxt(schemValidator); xmlSchemaFree(schemPtr); free(version); @@ -514,21 +516,21 @@ int ParseModelXML(Model_t *model) xmlDocument = xmlParseFile(model->filename); if (xmlDocument == NULL) { - printLog("Can't parse model file at '%s'.\n", model->filename); + printErr("Can't parse model file at '%s'.\n", model->filename); return -1; } currentNode = xmlDocGetRootElement(xmlDocument); if (currentNode == NULL) { - printLog("Invalid model file at '%s', document empty !\n", + printErr("Invalid model file at '%s', document empty !\n", model->filename); xmlFreeDoc(xmlDocument); return -2; } if (xmlStrcmp(currentNode->name, (const xmlChar *) "gem-graph-model")) { - printLog("Invalid model file at '%s', " + printErr("Invalid model file at '%s', " "root node is not !\n", model->filename); xmlFreeDoc(xmlDocument); diff --git a/src/scheduler.c b/src/scheduler.c index c159bf4..82a9268 100644 --- a/src/scheduler.c +++ b/src/scheduler.c @@ -105,8 +105,8 @@ static void *schedulerMain(void *scheduler) centersList = (Center_t*) calloc(1, sizeof(Center_t)); // Initiate the arrowArray lock - if (err = ArrowsInitLock(args->arrowArray), err != 0) { // TODO destroy this - printLog("Impossible to create the arrow array lock (error %d)\n", err); + if (err = ArrowsInitLock(args->arrowArray), err != 0) { + printErr("Impossible to create the arrow array lock (error %d)\n", err); return NULL; } // @@ -215,6 +215,9 @@ static void *schedulerMain(void *scheduler) printLog("Scheduler #%lu offline\n", args->id); + + ArrowsDestroyLock(args->arrowArray); + free(workerArray); workerArray = NULL; free(centersList); diff --git a/src/server.c b/src/server.c index f9986b4..400012e 100644 --- a/src/server.c +++ b/src/server.c @@ -75,7 +75,7 @@ void *serverCommunicationInstance(void *server) // Read the message from client and copy it in buffer bytesReceived = recv(args->sockfd, receiveBuff, RECEIVE_BUFFER_SIZE, 0); if (bytesReceived == -1) { - printLog("Could not receive data! (%s)\n", strerror(errno)); + printErr("Could not receive data! (%s)\n", strerror(errno)); break; }; @@ -148,7 +148,7 @@ static void *serverMain(void *server) // Create socket args->sockfd = socket(AF_INET, SOCK_STREAM, 0); if (args->sockfd == -1) { - printLog("Socket creation failed! (%s)\n", strerror(errno)); + printErr("Socket creation failed! (%s)\n", strerror(errno)); args->returnValue = 1; goto serverExiting; } @@ -156,14 +156,14 @@ static void *serverMain(void *server) // Get socket flags flags = fcntl(args->sockfd, F_GETFL); if (flags == -1) { - printLog("Socket parameters getting failed! (%s)\n", strerror(errno)); + printErr("Socket parameters getting failed! (%s)\n", strerror(errno)); args->returnValue = 1; goto serverExiting; } // Change socket flags to non-blocking if (fcntl(args->sockfd, F_SETFL, flags | O_NONBLOCK) < 0) { - printLog("Socket non-blocking setting failed! (%s)\n", strerror(errno)); + printErr("Socket non-blocking setting failed! (%s)\n", strerror(errno)); args->returnValue = 1; goto serverExiting; } @@ -179,14 +179,14 @@ static void *serverMain(void *server) // Binding newly created socket if ((bind(args->sockfd, (struct sockaddr*)&servaddr, sizeof(servaddr))) == -1) { - printLog("Socket bind failed! (%s)\n", strerror(errno)); + printErr("Socket bind failed! (%s)\n", strerror(errno)); args->returnValue = 1; goto serverExiting; } // Now server is ready to listen and verification if (listen(args->sockfd, MAX_CONNECTION) == -1) { - printLog("Socket listening failed! (%s)\n", strerror(errno)); + printErr("Socket listening failed! (%s)\n", strerror(errno)); args->returnValue = 1; goto serverExiting; } @@ -196,7 +196,7 @@ static void *serverMain(void *server) // Get server socket address structure if (getsockname(args->sockfd, (struct sockaddr *)&servaddr, &socklen) == -1) { - printLog("Could not get socket structure! (%s)\n", strerror(errno)); + printErr("Could not get socket structure! (%s)\n", strerror(errno)); args->returnValue = 1; goto serverExiting; } @@ -212,7 +212,7 @@ static void *serverMain(void *server) if (connfd < 0) { // If error is not due to lack of clients connecting, this is error if (errno != EWOULDBLOCK && errno != EAGAIN) { - printLog("Server acccept failed! (%s)\n", strerror(errno)); + printErr("Server acccept failed! (%s)\n", strerror(errno)); goto serverExiting; } sleep(1); @@ -237,7 +237,7 @@ static void *serverMain(void *server) serverCommunicationInstance, (void*)&serverSlots[serverSlotIndex]); if(threadStatus != 0) { - printLog("Error from pthread: %d (%s)\n", + printErr("Error from pthread: %d (%s)\n", threadStatus, strerror(errno)); args->returnValue = 1; goto serverExiting;