diff --git a/common/nutstream.cpp b/common/nutstream.cpp index 01471e1cf8..d4dc867a80 100644 --- a/common/nutstream.cpp +++ b/common/nutstream.cpp @@ -197,6 +197,7 @@ NutStream::status_t NutMemory::putData(const std::string & data) { * by packager who knows their system. */ static bool checkExistsWritableDir(const char *s) { + DIR *pd; #ifdef DEBUG std::cerr << "checkExistsWritableDir(" << (s ? s : "") << "): "; #endif @@ -207,12 +208,13 @@ static bool checkExistsWritableDir(const char *s) { return false; } - if (!opendir(s)) { + if (!(pd = opendir(s))) { #ifdef DEBUG std::cerr << "not a dir" << std::endl; #endif return false; } + closedir(pd); /* POSIX: If the requested access is permitted, access() succeeds * and shall return 0; otherwise, -1 shall be returned and errno diff --git a/configure.ac b/configure.ac index 1b6f8a330c..fa2021ba58 100644 --- a/configure.ac +++ b/configure.ac @@ -5799,6 +5799,7 @@ m4_foreach_w([SCRIPTFILE], [ scripts/Solaris/postremove scripts/Solaris/preproto.pl scripts/Solaris/nut + scripts/valgrind/valgrind.sh tools/gitlog2changelog.py tools/nut-snmpinfo.py ], [ diff --git a/drivers/generic_gpio_common.c b/drivers/generic_gpio_common.c index c4aa8ac90a..1d99db421a 100644 --- a/drivers/generic_gpio_common.c +++ b/drivers/generic_gpio_common.c @@ -35,7 +35,7 @@ struct gpioups_t *generic_gpio_open(const char *chipName); #ifndef DRIVERS_MAIN_WITHOUT_MAIN static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ -void generic_gpio_close(struct gpioups_t *gpioupsfd); +void generic_gpio_close(struct gpioups_t **gpioupsfdptr); #ifndef DRIVERS_MAIN_WITHOUT_MAIN static @@ -60,7 +60,7 @@ int calc_rule_states(int upsLinesStates[], int cRules[], int subCount, int sInde #ifndef DRIVERS_MAIN_WITHOUT_MAIN static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ -void update_ups_states(struct gpioups_t *gpioupsfd); +void update_ups_states(struct gpioups_t *gpioupsfdlocal); /* * allocate common data structures and process/check rules @@ -69,14 +69,18 @@ void update_ups_states(struct gpioups_t *gpioupsfd); static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ struct gpioups_t *generic_gpio_open(const char *chipName) { - struct gpioups_t *upsfdlocal = xcalloc(1, sizeof(*upsfdlocal)); - upsfdlocal->runOptions=0; /* don't use ROPT_REQRES and ROPT_EVMODE yet */ - upsfdlocal->chipName=chipName; + char *rules = getval("rules"); + struct gpioups_t *upsfdlocal = NULL; - if(!testvar("rules")) /* rules is required configuration parameter */ + if (!rules) /* rules is required configuration parameter */ fatalx(EXIT_FAILURE, "UPS status calculation rules not specified"); - get_ups_rules(upsfdlocal, (unsigned char *)getval("rules")); + upsfdlocal = xcalloc(1, sizeof(*upsfdlocal)); + + upsfdlocal->runOptions = 0; /* don't use ROPT_REQRES and ROPT_EVMODE yet */ + upsfdlocal->chipName = chipName; + + get_ups_rules(upsfdlocal, (unsigned char *)rules); upsfdlocal->upsLinesStates = xcalloc(upsfdlocal->upsLinesCount, sizeof(int)); return upsfdlocal; @@ -88,22 +92,23 @@ struct gpioups_t *generic_gpio_open(const char *chipName) { #ifndef DRIVERS_MAIN_WITHOUT_MAIN static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ -void generic_gpio_close(struct gpioups_t *gpioupsfdlocal) { - if (gpioupsfdlocal) { - if (gpioupsfdlocal->upsLines) { - free(gpioupsfdlocal->upsLines); +void generic_gpio_close(struct gpioups_t **gpioupsfdptr) { + if (gpioupsfdptr && *gpioupsfdptr) { + if ((*gpioupsfdptr)->upsLines) { + free((*gpioupsfdptr)->upsLines); } - if (gpioupsfdlocal->upsLinesStates) { - free(gpioupsfdlocal->upsLinesStates); + if ((*gpioupsfdptr)->upsLinesStates) { + free((*gpioupsfdptr)->upsLinesStates); } - if (gpioupsfdlocal->rules) { - int i; - for (i=0; i < gpioupsfdlocal->rulesCount; i++) { - free(gpioupsfdlocal->rules[i]); + if ((*gpioupsfdptr)->rules) { + int i; + for (i = 0; i < (*gpioupsfdptr)->rulesCount; i++) { + free((*gpioupsfdptr)->rules[i]); } + free((*gpioupsfdptr)->rules); } - free(gpioupsfdlocal); - gpioupsfdlocal = NULL; + free(*gpioupsfdptr); + *gpioupsfdptr = NULL; } } @@ -114,12 +119,12 @@ void generic_gpio_close(struct gpioups_t *gpioupsfdlocal) { static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ void add_rule_item(struct gpioups_t *upsfdlocal, int newValue) { - int subCount = (upsfdlocal->rules[upsfdlocal->rulesCount-1]) ? upsfdlocal->rules[upsfdlocal->rulesCount-1]->subCount+1 : 1; - int itemSize = subCount*sizeof(upsfdlocal->rules[0]->cRules[0])+sizeof(rulesint); + int subCount = (upsfdlocal->rules[upsfdlocal->rulesCount - 1]) ? upsfdlocal->rules[upsfdlocal->rulesCount - 1]->subCount + 1 : 1; + int itemSize = subCount * sizeof(upsfdlocal->rules[0]->cRules[0]) + sizeof(rulesint); - upsfdlocal->rules[upsfdlocal->rulesCount-1] = xrealloc(upsfdlocal->rules[upsfdlocal->rulesCount-1], itemSize); - upsfdlocal->rules[upsfdlocal->rulesCount-1]->subCount = subCount; - upsfdlocal->rules[upsfdlocal->rulesCount-1]->cRules[subCount-1] = newValue; + upsfdlocal->rules[upsfdlocal->rulesCount - 1] = xrealloc(upsfdlocal->rules[upsfdlocal->rulesCount - 1], itemSize); + upsfdlocal->rules[upsfdlocal->rulesCount - 1]->subCount = subCount; + upsfdlocal->rules[upsfdlocal->rulesCount - 1]->cRules[subCount - 1] = newValue; } /* @@ -131,7 +136,7 @@ void add_rule_item(struct gpioups_t *upsfdlocal, int newValue) { static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ int get_rule_lex(unsigned char *rulesBuff, int *startPos, int *endPos) { - static unsigned char lexType[256]={ + static unsigned char lexType[256] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 00 0x00 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 16 0x10 */ 0, 0, 0, 0, 0, 0,'&', 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 32 0x20 */ @@ -149,11 +154,11 @@ int get_rule_lex(unsigned char *rulesBuff, int *startPos, int *endPos) { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 224 0xe0 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 /* 240 0xf0 */ }; - unsigned char lexTypeChr = lexType[rulesBuff[*startPos]]; + unsigned char lexTypeChr = lexType[rulesBuff[*startPos]]; - *endPos = (*startPos)+1; - if(lexTypeChr == 'a' || lexTypeChr == '0') { - for(; lexType[rulesBuff[*endPos]] == lexTypeChr; (*endPos)++); + *endPos = (*startPos) + 1; + if (lexTypeChr == 'a' || lexTypeChr == '0') { + for (; lexType[rulesBuff[*endPos]] == lexTypeChr; (*endPos)++); } return (int)lexTypeChr; } @@ -165,19 +170,19 @@ int get_rule_lex(unsigned char *rulesBuff, int *startPos, int *endPos) { static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { - /* statename=[^]line[&||[line]] */ - char lexBuff[33]; - int startPos = 0, endPos; - int lexType; - int lexStatus = 0; + /* statename = [^]line[&||[line]] */ + char lexBuff[33]; + int startPos = 0, endPos; + int lexType; + int lexStatus = 0; int i, j, k; int tranformationDelta; upsdebugx(4, "rules = [%s]", rulesString); /* state machine to process rules definition */ - while((lexType=get_rule_lex(rulesString, &startPos, &endPos)) > 0 && lexStatus >= 0) { + while ((lexType = get_rule_lex(rulesString, &startPos, &endPos)) > 0 && lexStatus >= 0) { memset(lexBuff, 0, sizeof(lexBuff)); - strncpy(lexBuff, (char *)(rulesString+startPos), endPos-startPos); + strncpy(lexBuff, (char *)(rulesString + startPos), endPos - startPos); upsdebugx(4, "rules start %d, end %d, lexType %d, lex [%s]", startPos, @@ -187,20 +192,20 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { ); switch(lexStatus) { case 0: - if(lexType != 'a') { - lexStatus=-1; + if (lexType != 'a') { + lexStatus = -1; } else { lexStatus = 1; upsfdlocal->rulesCount++; upsfdlocal->rules = xrealloc(upsfdlocal->rules, (size_t)(sizeof(upsfdlocal->rules[0])*upsfdlocal->rulesCount)); - upsfdlocal->rules[upsfdlocal->rulesCount-1] = xcalloc(1, sizeof(rulesint)); - strncpy(upsfdlocal->rules[upsfdlocal->rulesCount-1]->stateName, (char *)(rulesString+startPos), endPos-startPos); - upsfdlocal->rules[upsfdlocal->rulesCount-1]->stateName[endPos-startPos] = 0; + upsfdlocal->rules[upsfdlocal->rulesCount -1 ] = xcalloc(1, sizeof(rulesint)); + strncpy(upsfdlocal->rules[upsfdlocal->rulesCount - 1]->stateName, (char *)(rulesString + startPos), endPos - startPos); + upsfdlocal->rules[upsfdlocal->rulesCount - 1]->stateName[endPos - startPos] = 0; } break; case 1: - if(lexType != '=') { + if (lexType != '=') { lexStatus = -1; } else { lexStatus = 2; @@ -208,35 +213,35 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { break; case 2: - if(lexType == '^') { + if (lexType == '^') { lexStatus = 3; add_rule_item(upsfdlocal, RULES_CMD_NOT); - } else if(lexType == '0') { + } else if (lexType == '0') { lexStatus = 4; - add_rule_item(upsfdlocal, atoi((char *)(rulesString+startPos))); + add_rule_item(upsfdlocal, atoi((char *)(rulesString + startPos))); } else { lexStatus = -1; } break; case 3: - if(lexType != '0') { + if (lexType != '0') { lexStatus = -1; } else { lexStatus = 4; - add_rule_item(upsfdlocal, atoi((char *)(rulesString+startPos))); + add_rule_item(upsfdlocal, atoi((char *)(rulesString + startPos))); } break; case 4: - if(lexType == '&') { + if (lexType == '&') { lexStatus = 2; add_rule_item(upsfdlocal, RULES_CMD_AND); - } else if(lexType == '|') { + } else if (lexType == '|') { lexStatus = 2; add_rule_item(upsfdlocal, RULES_CMD_OR); } - else if(lexType == ';') { + else if (lexType == ';') { lexStatus = 0; } else { lexStatus = -1; @@ -247,22 +252,22 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { lexStatus = -1; break; } - if(lexStatus == -1) + if (lexStatus == -1) fatalx(LOG_ERR, "Line processing rule error at position %d", startPos); startPos = endPos; } - if(lexType == 0 && lexStatus != 0) + if (lexType == 0 && lexStatus != 0) fatalx(LOG_ERR, "Line processing rule error at position %d", startPos); /* debug printout for extracted rules */ upsdebugx(4, "rules count [%d]", upsfdlocal->rulesCount); - for(i = 0; i < upsfdlocal->rulesCount; i++) { + for (i = 0; i < upsfdlocal->rulesCount; i++) { upsdebugx(4, "rule state name [%s], subcount %d", upsfdlocal->rules[i]->stateName, upsfdlocal->rules[i]->subCount ); - for(j = 0; jrules[i]->subCount; j++) { + for (j = 0; jrules[i]->subCount; j++) { upsdebugx(4, "[%s] substate %d [%d]", upsfdlocal->rules[i]->stateName, @@ -275,21 +280,21 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { /* get gpio lines used in rules, find max line number used to check with chip lines count*/ upsfdlocal->upsLinesCount = 0; upsfdlocal->upsMaxLine = 0; - for(i = 0; i < upsfdlocal->rulesCount; i++) { - for(j = 0; j < upsfdlocal->rules[i]->subCount; j++) { - int pinOnList = 0; - for(k = 0; k < upsfdlocal->upsLinesCount && !pinOnList; k++) { - if(upsfdlocal->upsLines[k] == upsfdlocal->rules[i]->cRules[j]) { + for (i = 0; i < upsfdlocal->rulesCount; i++) { + for (j = 0; j < upsfdlocal->rules[i]->subCount; j++) { + int pinOnList = 0; + for (k = 0; k < upsfdlocal->upsLinesCount && !pinOnList; k++) { + if (upsfdlocal->upsLines[k] == upsfdlocal->rules[i]->cRules[j]) { pinOnList = 1; } } - if(!pinOnList) { - if(upsfdlocal->rules[i]->cRules[j] >= 0) { + if (!pinOnList) { + if (upsfdlocal->rules[i]->cRules[j] >= 0) { upsfdlocal->upsLinesCount++; upsfdlocal->upsLines = xrealloc(upsfdlocal->upsLines, sizeof(upsfdlocal->upsLines[0])*upsfdlocal->upsLinesCount); - upsfdlocal->upsLines[upsfdlocal->upsLinesCount-1] = upsfdlocal->rules[i]->cRules[j]; - if(upsfdlocal->upsLines[upsfdlocal->upsLinesCount-1] > upsfdlocal->upsMaxLine) { - upsfdlocal->upsMaxLine = upsfdlocal->upsLines[upsfdlocal->upsLinesCount-1]; + upsfdlocal->upsLines[upsfdlocal->upsLinesCount - 1] = upsfdlocal->rules[i]->cRules[j]; + if (upsfdlocal->upsLines[upsfdlocal->upsLinesCount - 1] > upsfdlocal->upsMaxLine) { + upsfdlocal->upsMaxLine = upsfdlocal->upsLines[upsfdlocal->upsLinesCount - 1]; } } } @@ -297,23 +302,23 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { } upsdebugx(4, "UPS line count = %d", upsfdlocal->upsLinesCount); - for(i = 0; i < upsfdlocal->upsLinesCount; i++) { + for (i = 0; i < upsfdlocal->upsLinesCount; i++) { upsdebugx(4, "UPS line%d number %d", i, upsfdlocal->upsLines[i]); } /* transform lines to indexes for easier state calculation */ tranformationDelta = upsfdlocal->upsMaxLine - RULES_CMD_LAST + 1; - for(i = 0; i < upsfdlocal->rulesCount; i++) { - for(j = 0; j < upsfdlocal->rules[i]->subCount; j++) { - if(upsfdlocal->rules[i]->cRules[j] >= 0) { + for (i = 0; i < upsfdlocal->rulesCount; i++) { + for (j = 0; j < upsfdlocal->rules[i]->subCount; j++) { + if (upsfdlocal->rules[i]->cRules[j] >= 0) { upsfdlocal->rules[i]->cRules[j] -= tranformationDelta; } } } - for(k = 0; k < upsfdlocal->upsLinesCount; k++) { - for(i = 0; i < upsfdlocal->rulesCount; i++) { - for(j = 0; j < upsfdlocal->rules[i]->subCount; j++) { - if((upsfdlocal->rules[i]->cRules[j] + tranformationDelta) == upsfdlocal->upsLines[k]) { + for (k = 0; k < upsfdlocal->upsLinesCount; k++) { + for (i = 0; i < upsfdlocal->rulesCount; i++) { + for (j = 0; j < upsfdlocal->rules[i]->subCount; j++) { + if ((upsfdlocal->rules[i]->cRules[j] + tranformationDelta) == upsfdlocal->upsLines[k]) { upsfdlocal->rules[i]->cRules[j] = k; } } @@ -322,13 +327,13 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { /* debug printout of transformed lines numbers */ upsdebugx(4, "rules count [%d] translated", upsfdlocal->rulesCount); - for(i = 0; i < upsfdlocal->rulesCount; i++) { + for (i = 0; i < upsfdlocal->rulesCount; i++) { upsdebugx(4, "rule state name [%s], subcount %d translated", upsfdlocal->rules[i]->stateName, upsfdlocal->rules[i]->subCount ); - for(j = 0; j < upsfdlocal->rules[i]->subCount; j++) { + for (j = 0; j < upsfdlocal->rules[i]->subCount; j++) { upsdebugx(4, "[%s] substate %d [%d]", upsfdlocal->rules[i]->stateName, j, @@ -345,11 +350,12 @@ void get_ups_rules(struct gpioups_t *upsfdlocal, unsigned char *rulesString) { static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ int calc_rule_states(int upsLinesStates[], int cRules[], int subCount, int sIndex) { - int ruleVal = 0; - int iopStart = sIndex; - int rs; - if(iopStart < subCount) { /* calculate left side */ - if(cRules[iopStart] >= 0) { + int ruleVal = 0; + int iopStart = sIndex; + int rs; + + if (iopStart < subCount) { /* calculate left side */ + if (cRules[iopStart] >= 0) { ruleVal = upsLinesStates[cRules[iopStart]]; } else { iopStart++; @@ -357,13 +363,14 @@ int calc_rule_states(int upsLinesStates[], int cRules[], int subCount, int sInde } iopStart++; } - for(; iopStart < subCount; iopStart++) { /* right side calculation */ - if(cRules[iopStart] == RULES_CMD_OR) { - ruleVal = ruleVal || calc_rule_states(upsLinesStates, cRules, subCount, iopStart+1); + + for (; iopStart < subCount; iopStart++) { /* right side calculation */ + if (cRules[iopStart] == RULES_CMD_OR) { + ruleVal = ruleVal || calc_rule_states(upsLinesStates, cRules, subCount, iopStart + 1); break; } else { iopStart++; - if(cRules[iopStart] == RULES_CMD_NOT) { + if (cRules[iopStart] == RULES_CMD_NOT) { iopStart++; rs = !upsLinesStates[cRules[iopStart]]; } else { @@ -384,39 +391,39 @@ int calc_rule_states(int upsLinesStates[], int cRules[], int subCount, int sInde static #endif /* DRIVERS_MAIN_WITHOUT_MAIN */ void update_ups_states(struct gpioups_t *gpioupsfdlocal) { - int batLow = 0; - int bypass = 0; - int chargerStatusSet = 0; - int ruleNo; + int batLow = 0; + int bypass = 0; + int chargerStatusSet = 0; + int ruleNo; status_init(); - for(ruleNo = 0; ruleNo < gpioupsfdlocal->rulesCount; ruleNo++) { + for (ruleNo = 0; ruleNo < gpioupsfdlocal->rulesCount; ruleNo++) { gpioupsfdlocal->rules[ruleNo]->currVal = calc_rule_states( gpioupsfdlocal->upsLinesStates, gpioupsfdlocal->rules[ruleNo]->cRules, gpioupsfdlocal->rules[ruleNo]->subCount, 0 ); - if(gpioupsfdlocal->rules[ruleNo]->currVal) { + if (gpioupsfdlocal->rules[ruleNo]->currVal) { status_set(gpioupsfdlocal->rules[ruleNo]->stateName); - if(!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "CHRG")) { + if (!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "CHRG")) { dstate_setinfo("battery.charger.status", "%s", "charging"); chargerStatusSet++; } - if(!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "DISCHRG")) { + if (!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "DISCHRG")) { dstate_setinfo("battery.charger.status", "%s", "discharging"); chargerStatusSet++; } - if(!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "LB")) { + if (!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "LB")) { batLow = 1; } - if(!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "BYPASS")) { + if (!strcmp(gpioupsfdlocal->rules[ruleNo]->stateName, "BYPASS")) { bypass = 1; } } - if(gpioupsfdlocal->aInfoAvailable && + if (gpioupsfdlocal->aInfoAvailable && gpioupsfdlocal->rules[ruleNo]->archVal != gpioupsfdlocal->rules[ruleNo]->currVal) { upslogx(LOG_WARNING, "UPS state [%s] changed to %d", gpioupsfdlocal->rules[ruleNo]->stateName, @@ -426,19 +433,19 @@ void update_ups_states(struct gpioups_t *gpioupsfdlocal) { gpioupsfdlocal->rules[ruleNo]->archVal = gpioupsfdlocal->rules[ruleNo]->currVal; } - if(chargerStatusSet <= 0) { + if (chargerStatusSet <= 0) { dstate_delinfo("battery.charger.status"); } - if(dstate_getinfo("battery.charge.low")!=NULL) { - if(batLow) { + if (dstate_getinfo("battery.charge.low") != NULL) { + if (batLow) { dstate_setinfo("battery.charge", "%s", dstate_getinfo("battery.charge.low")); } else { dstate_setinfo("battery.charge", "%s", "100"); } } - if(bypass) { + if (bypass) { dstate_delinfo("battery.charge"); } @@ -449,10 +456,10 @@ void update_ups_states(struct gpioups_t *gpioupsfdlocal) { void upsdrv_initinfo(void) { - if(testvar("mfr")) { + if (testvar("mfr")) { dstate_setinfo("device.mfr", "%s", getval("mfr")); } - if(testvar("model")) { + if (testvar("model")) { dstate_setinfo("device.model", "%s", getval("model")); } } @@ -495,17 +502,19 @@ void upsdrv_makevartable(void) void upsdrv_initups(void) { /* prepare rules and allocate related structures */ - gpioupsfd=generic_gpio_open(device_path); + gpioupsfd = generic_gpio_open(device_path); /* open GPIO chip and check pin consistence */ - if(gpioupsfd) { + if (gpioupsfd) { gpio_open(gpioupsfd); } } void upsdrv_cleanup(void) { - /* release gpio library resources */ - gpio_close(gpioupsfd); - /* release related generic resources */ - generic_gpio_close(gpioupsfd); + if (gpioupsfd) { + /* release gpio library resources */ + gpio_close(gpioupsfd); + /* release related generic resources */ + generic_gpio_close(&gpioupsfd); + } } diff --git a/drivers/generic_gpio_common.h b/drivers/generic_gpio_common.h index 5589fc7ec7..7e92a8965e 100644 --- a/drivers/generic_gpio_common.h +++ b/drivers/generic_gpio_common.h @@ -75,7 +75,7 @@ void gpio_close(struct gpioups_t *gpioupsfd); # ifdef DRIVERS_MAIN_WITHOUT_MAIN /* Methods externalized for unit-tests, otherwise private to this module */ struct gpioups_t *generic_gpio_open(const char *chipName); -void generic_gpio_close(struct gpioups_t *gpioupsfd); +void generic_gpio_close(struct gpioups_t **gpioupsfdptr); void get_ups_rules(struct gpioups_t *upsfd, unsigned char *rulesString); void add_rule_item(struct gpioups_t *upsfd, int newValue); int get_rule_lex(unsigned char *rulesBuff, int *startPos, int *endPos); diff --git a/scripts/valgrind/.gitignore b/scripts/valgrind/.gitignore new file mode 100644 index 0000000000..83ee3f6e3f --- /dev/null +++ b/scripts/valgrind/.gitignore @@ -0,0 +1 @@ +/valgrind.sh diff --git a/scripts/valgrind/.valgrind.supp b/scripts/valgrind/.valgrind.supp index 2e175ebaf5..ea2866c880 100644 --- a/scripts/valgrind/.valgrind.supp +++ b/scripts/valgrind/.valgrind.supp @@ -1,3 +1,22 @@ +{ + + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + obj:/usr/bin/dash + ... + fun:(below main) +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + fun:strdup + obj:/usr/bin/dash + ... + fun:(below main) +} { Memcheck:Param @@ -140,3 +159,177 @@ fun:start_thread ... } + +# Numerous reported leaks seem to be part of CPPUNIT itself, quesce them: +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::ConcretTestFixtureFactory<*>::makeFixture() + ... + #CppUnit::TestFactoryRegistry::makeTest() + fun:_ZN7CppUnit19TestFactoryRegistry8makeTestEv + ... +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::SynchronizedObject::SynchronizedObject(CppUnit::SynchronizedObject::SynchronizationObject*) + fun:_ZN7CppUnit18SynchronizedObjectC1EPNS0_21SynchronizationObjectE + ... +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TestResult::TestResult(CppUnit::SynchronizedObject::SynchronizationObject*) + fun:_ZN7CppUnit10TestResultC1EPNS_18SynchronizedObject21SynchronizationObjectE + ... +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TestRunner::TestRunner() + fun:_ZN7CppUnit10TestRunnerC1Ev + #CppUnit::TextTestRunner::TextTestRunner(CppUnit::Outputter*) + fun:_ZN7CppUnit14TextTestRunnerC1EPNS_9OutputterE + fun:main +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TextTestRunner::TextTestRunner(CppUnit::Outputter*) + fun:_ZN7CppUnit14TextTestRunnerC1EPNS_9OutputterE + fun:main +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TestResultCollector::TestResultCollector(CppUnit::SynchronizedObject::SynchronizationObject*) + fun:_ZN7CppUnit19TestResultCollectorC1EPNS_18SynchronizedObject21SynchronizationObjectE + #CppUnit::TextTestRunner::TextTestRunner(CppUnit::Outputter*) + fun:_ZN7CppUnit14TextTestRunnerC1EPNS_9OutputterE + fun:main +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TestSuite::addTest(CppUnit::Test*) + fun:_ZN7CppUnit9TestSuite7addTestEPNS_4TestE + fun:main +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TestComposite::TestComposite(std::__cxx11::basic_string, std::allocator > const&) + fun:_ZN7CppUnit13TestCompositeC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE + ... +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::TestCase::TestCase(std::__cxx11::basic_string, std::allocator > const&) + fun:_ZN7CppUnit8TestCaseC1ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE + ... +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #void std::_Function_base::_Base_manager >::_M_create >(std::_Any_data&, std::_Bind&&, std::integral_constant) + ###fun:_ZNSt14_Function_base13_Base_managerISt5_BindIFM11ExampleTestFvvEPS2_EEE9_M_createIS7_EEvRSt9_Any_dataOT_St17integral_constantIbLb0EE + fun:_ZNSt14_Function_base13_Base_managerISt5_BindIFM11*vvEPS2_EEE9_M_createIS7_EEvRSt9_Any_dataOT_St17integral_constantIbLb0EE + ... + #CppUnit::TestFactoryRegistry::addTestToSuite(CppUnit::TestSuite*) + fun:_ZN7CppUnit19TestFactoryRegistry14addTestToSuiteEPNS_9TestSuiteE +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::ProtectorChain::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) + fun:_ZN7CppUnit14ProtectorChain7protectERKNS_7FunctorERKNS_16ProtectorContextE + #CppUnit::TestResult::protect(CppUnit::Functor const&, CppUnit::Test*, std::__cxx11::basic_string, std::allocator > const&) + fun:_ZN7CppUnit10TestResult7protectERKNS_7FunctorEPNS_4TestERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE + ... +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + #CppUnit::ProtectorChain::protect() + fun:_ZN7CppUnit14ProtectorChainC1Ev + #CppUnit::TestResult::TestResult(CppUnit::SynchronizedObject::SynchronizationObject*) + fun:_ZN7CppUnit10TestResultC1EPNS_18SynchronizedObject21SynchronizationObjectE + #CppUnit::TextTestRunner::TextTestRunner(CppUnit::Outputter*) + fun:_ZN7CppUnit14TextTestRunnerC1EPNS_9OutputterE + fun:main +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + obj:*libcppunit* + #CppUnit::TestResultCollector::TestResultCollector(CppUnit::SynchronizedObject::SynchronizationObject*) + fun:_ZN7CppUnit19TestResultCollectorC1EPNS_18SynchronizedObject21SynchronizationObjectE + #CppUnit::TextTestRunner::TextTestRunner(CppUnit::Outputter*) + fun:_ZN7CppUnit14TextTestRunnerC1EPNS_9OutputterE + fun:main +} +{ + + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + obj:*libcppunit* + #CppUnit::TestPath::TestPath(CppUnit::Test*, std::__cxx11::basic_string, std::allocator > const&) + fun:_ZN7CppUnit8TestPathC1EPNS_4TestERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE + #CppUnit::TestRunner::run(CppUnit::TestResult&, std::__cxx11::basic_string, std::allocator > const&) + fun:_ZN7CppUnit10TestRunner3runERNS_10TestResultERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE + #CppUnit::TextTestRunner::run(std::__cxx11::basic_string, std::allocator >, bool, bool, bool) + fun:_ZN7CppUnit14TextTestRunner3runENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEbbb + fun:main +} +{ + + ### runner.setOutputter( new CppUnit::CompilerOutputter( ...) ) in main (cpputest.cpp:120) + Memcheck:Leak + match-leak-kinds: reachable + #operator new(unsigned long) + fun:_Znwm + fun:main +} diff --git a/scripts/valgrind/README.adoc b/scripts/valgrind/README.adoc index 6d5866163e..bf55b224d6 100644 --- a/scripts/valgrind/README.adoc +++ b/scripts/valgrind/README.adoc @@ -9,6 +9,11 @@ Example use-case: ./scripts/valgrind/valgrind.sh ./tools/nut-scanner/nut-scanner -DDDDDD -m auto ---- +Note that the script is generated under `${top_builddir}` by `configure` from +a template file located in `${top_srcdir}/scripts/valgrind/valgrind.sh.in`. +You might be able to run it directly, falling back to a `valgrind` program in +your `PATH`, if any. + See also: * link:https://wiki.wxwidgets.org/Valgrind_Suppression_File_Howto[Valgrind Suppression File How-to] diff --git a/scripts/valgrind/valgrind.sh b/scripts/valgrind/valgrind.sh.in similarity index 51% rename from scripts/valgrind/valgrind.sh rename to scripts/valgrind/valgrind.sh.in index 2bb8608125..966c29f751 100755 --- a/scripts/valgrind/valgrind.sh +++ b/scripts/valgrind/valgrind.sh.in @@ -1,18 +1,26 @@ #!/bin/sh -# Copyright (C) 2024 by Jim Klimov +# Copyright (C) 2024-2025 by Jim Klimov +# NOTE: If there are system problems to suppress, re-run the test with +# --gen-suppressions=all option, and update the .valgrind.supp file. SCRIPTDIR="`dirname "$0"`" SCRIPTDIR="`cd "$SCRIPTDIR" && pwd`" LTEXEC="" +[ -n "${LIBTOOL-}" ] || LIBTOOL="`command -v glibtool`" 2>/dev/null [ -n "${LIBTOOL-}" ] || LIBTOOL="`command -v libtool`" 2>/dev/null [ -z "${LIBTOOL}" ] || LTEXEC="${LIBTOOL} --mode=execute " +# Allow to run un-parsed "sh ${top_srcdir}/scripts/valgrind/valgrind.sh.in" +# with reasonable success: +[ -n "${VALGRIND-}" ] || case '@VALGRIND@' in [@]*|none) ;; *) VALGRIND='@VALGRIND@' ;; esac +[ -n "${VALGRIND-}" ] || VALGRIND="valgrind" + # TODO: Also wrap tool=callgrind e.g. via $0 symlink name? exec ${LTEXEC} \ -valgrind \ +${VALGRIND} \ --tool=memcheck --verbose \ --leak-check=full --show-reachable=yes --error-exitcode=1 --show-error-list=yes \ --trace-children=yes --track-fds=yes --show-leak-kinds=all --track-origins=yes \ diff --git a/tests/Makefile.am b/tests/Makefile.am index 48beeac5fd..38b4f71c99 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -192,13 +192,25 @@ endif !HAVE_CXX11 if HAVE_VALGRIND # NOTE: "cppnit", if built, requires running from NIT (with NUT_PORT, etc.) +# Note that FAILED value begins with a space, so we do not echo another memcheck: $(check_PROGRAMS) - RES=0; for P in $? ; do \ + @RES=0; FAILED=""; \ + for P in $? ; do \ case "$$P" in \ - cppnit|cppnit$(EXEEXT)) if [ "$${NUT_PORT-}" -gt 0 ] 2>/dev/null ; then : ; else echo "SKIPPED: $(VALGRIND) ./$$P : NUT_PORT not prepared" ; continue ; fi ;; \ + cppnit|cppnit$(EXEEXT)) \ + if [ "$${NUT_PORT-}" -gt 0 ] 2>/dev/null ; then : ; else \ + echo "SKIPPED: $(VALGRIND) ./$$P : NUT_PORT not prepared" ; \ + continue ; \ + fi ;; \ esac ; \ - $(VALGRIND) ./$$P || { RES=$$? ; echo "FAILED: $(VALGRIND) ./$$P" >&2; } ; \ - done; exit $$RES + '$(top_builddir)/scripts/valgrind/valgrind.sh' "./$$P" \ + || { RES=$$? ; \ + echo "FAILED: '$(top_builddir)/scripts/valgrind/valgrind.sh' './$$P'" >&2; \ + FAILED="$$FAILED $$P" ; \ + } ; \ + done; \ + if [ -n "$$FAILED" ] ; then echo "OVERALL FAILED:$$FAILED" >&2 ; fi ; \ + exit $$RES else !HAVE_VALGRIND memcheck: @echo "SKIPPED $@ : valgrind was not detected on this system by configure script" >&2 diff --git a/tests/generic_gpio_utest.c b/tests/generic_gpio_utest.c index 35759d50eb..3c171672df 100644 --- a/tests/generic_gpio_utest.c +++ b/tests/generic_gpio_utest.c @@ -101,7 +101,7 @@ int get_test_status(struct gpioups_t *result, int on_fail_path) { for (i=0; irulesCount; i++) { fEof=fscanf(testData, "%s", stateName); - if(!strcmp(result->rules[i]->stateName,stateName)) { + if(strcmp(result->rules[i]->stateName,stateName)) { cases_failed++; printf("expecting stateName %s, got %s for rule %d\n", stateName, result->rules[i]->stateName, i); return 1; @@ -211,15 +211,15 @@ int main(int argc, char **argv) { if(fEof!=EOF) { if(!strcmp(testType, "rules")) { struct gpioups_t *upsfdtest = xcalloc(1, sizeof(*upsfdtest)); + /* NOTE: here and below, freed by generic_gpio_close(&upsfdtest) */ jmp_result = setjmp(env_buffer); if(jmp_result) { /* test case exiting */ - generic_gpio_close(upsfdtest); printf("%s %s test rule %u [%s]\n", pass_fail[get_test_status(upsfdtest, 1)], testType, i, rules); } else { /* run test case */ get_ups_rules(upsfdtest, (unsigned char *)rules); - generic_gpio_close(upsfdtest); printf("%s %s test rule %u [%s]\n", pass_fail[get_test_status(upsfdtest, 0)], testType, i, rules); } + generic_gpio_close(&upsfdtest); } if(!strcmp(testType, "states")) { int expectedStateValue; @@ -248,7 +248,7 @@ int main(int argc, char **argv) { cases_failed++; } } - generic_gpio_close(upsfdtest); + generic_gpio_close(&upsfdtest); } if(!strcmp(testType, "update")) { char upsStatus[256]; @@ -281,7 +281,6 @@ int main(int argc, char **argv) { jmp_result = setjmp(env_buffer); if (jmp_result) { failed=1; - generic_gpio_close(upsfdtest); } else { update_ups_states(upsfdtest); currUpsStatus=dstate_getinfo("ups.status"); @@ -292,8 +291,8 @@ int main(int argc, char **argv) { if(!strcmp(chargeStatus,".") && currChargerStatus!=NULL) failed=1; if( strcmp(chargeLow,".") && strcmp(charge,".") && (!currCharge || strcmp(currCharge, charge))) failed=1; if(!strcmp(chargeLow,".") && !strcmp(charge,".") && currCharge!=NULL) failed=1; - generic_gpio_close(upsfdtest); } + generic_gpio_close(&upsfdtest); printf("%s %s test rule %u [%s] ([%s] %s %s (%s)) ([%s] %s %s)\n", pass_fail[failed], testType, i, rules, upsStatus, chargeStatus, charge, chargeLow, @@ -390,6 +389,11 @@ int main(int argc, char **argv) { fclose(testData); done = 1; + dstate_free(); + /* Should be safe if we happen to run this twice for + * generic_gpio_common.c, it only frees driver variables once */ + upsdrv_cleanup(); + /* Return 0 (exit-code OK, boolean false) if no tests failed and some ran */ if ( (cases_failed == 0) && (cases_passed > 0) ) return 0; diff --git a/tests/nutconf_ut.cpp b/tests/nutconf_ut.cpp index 4c135ae5aa..715676e4ab 100644 --- a/tests/nutconf_ut.cpp +++ b/tests/nutconf_ut.cpp @@ -196,6 +196,7 @@ void NutConfigUnitTest::testNutConfiguration() { void NutConfigUnitTest::testUpsmonConfiguration() { nut::UpsmonConfiguration config; + nut::BoolInt *tmpPtr = nullptr; // Note: this file gets generated from a .in template load(static_cast(&config), ABS_TOP_BUILDDIR "/conf/upsmon.conf.sample"); @@ -212,10 +213,10 @@ void NutConfigUnitTest::testUpsmonConfiguration() { // Hm, maybe we need typed constructors to init // the values for test? ("new" weirdness below // is due to Settable<> formal type mismatch) - config.shutdownExit = (*(new nut::BoolInt()) << "true"); + config.shutdownExit = (*(tmpPtr = new nut::BoolInt()) << "true"); delete tmpPtr; config.oblbDuration = -1; - config.certVerify = (*(new nut::BoolInt()) << false); - config.forceSsl = (*(new nut::BoolInt()) << "1"); + config.certVerify = (*(tmpPtr = new nut::BoolInt()) << false); delete tmpPtr; + config.forceSsl = (*(tmpPtr = new nut::BoolInt()) << "1"); delete tmpPtr; config.certIdent.certName = "My test cert"; config.certIdent.certDbPass = "DbPwd!"; @@ -223,14 +224,14 @@ void NutConfigUnitTest::testUpsmonConfiguration() { nut::CertHost certHost; certHost.host = "remote:3493"; certHost.certName = "NUT server cert"; - certHost.certVerify = (*(new nut::BoolInt()) << "true"); - certHost.forceSsl = (*(new nut::BoolInt()) << 0); + certHost.certVerify = (*(tmpPtr = new nut::BoolInt()) << "true"); delete tmpPtr; + certHost.forceSsl = (*(tmpPtr = new nut::BoolInt()) << 0); delete tmpPtr; config.certHosts.push_back(certHost); certHost.host = "localhost:13493"; certHost.certName = "My NUT server cert"; - certHost.certVerify = (*(new nut::BoolInt()) << "false"); - certHost.forceSsl = (*(new nut::BoolInt()) << false); + certHost.certVerify = (*(tmpPtr = new nut::BoolInt()) << "false"); delete tmpPtr; + certHost.forceSsl = (*(tmpPtr = new nut::BoolInt()) << false); delete tmpPtr; config.certHosts.push_back(certHost); // Note: More config data points come from the file loaded above