You can go ahead and push. Ack from me too. Thanks, Mathi.
----- [email protected] wrote: > Hi Giang, > > I could apply the attached patch without problem. > > Ack, with a minor comment. > > In the option switch there is a "case '?':", this is meaningless > unless the option string also contains this character e.g. > "?hlnya:s:" > > If Mathi has no more comments I can fix this and push this patch. > > Thanks > Lennart > > > -----Original Message----- > From: giang do [mailto:[email protected]] > Sent: den 3 september 2015 09:27 > To: Mathivanan Naickan Palanivelu; Vu Nguyen M; Lennart Lund > Cc: [email protected] > Subject: Re: [PATCH 1 of 1] log: saflogger does not check invalid > options [#1367] > > Hi Mathi, > > I also tried to applied my patch that sent to you but I can not. > I don't know why. > > Please try my patch in attachment again. > > About freeing logBuf: > > Because string that pointed by logBug will be used later in > write_log_record(logHandle, logStreamHandle, selectionObject, > &logRecord), we can not free it immediately. > > Instead we should free logRecord.logBuffer->logBuf. > > But, because not freeing this pointer do not cause serious memory > leak, I think we can ignore it. > I've try to run valgrind and the the result just say that this is > "still reachable". > > Base on this discussion I think we can ignore freeing that pointer: > http://stackoverflow.com/questions/3840582/still-reachable-leak-detected-by-valgrind > > valgrind --show-reachable=yes --leak-check=full --track-origins=yes > /usr/local/bin/saflogger abc > > ==750== Thread 1: > ==750== 32 bytes in 1 blocks are still reachable in loss record 1 of > 2 > ==750== at 0x4C2C494: calloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==750== by 0x589768F: _dlerror_run (dlerror.c:141) > ==750== by 0x58970C0: dlopen@@GLIBC_2.2.5 (dlopen.c:87) > ==750== by 0x5066FB4: ncs_leap_startup (ncs_main_pub.c:254) > ==750== by 0x5067306: ncs_core_agents_startup (ncs_main_pub.c:337) > ==750== by 0x5067448: ncs_agents_startup (ncs_main_pub.c:180) > ==750== by 0x52C9564: lga_startup (lga_util.c:282) > ==750== by 0x52C799D: saLogInitialize (lga_api.c:131) > ==750== by 0x401495: main (saf_logger.c:368) > ==750== > ==750== 67 bytes in 1 blocks are still reachable in loss record 2 of > 2 > ==750== at 0x4C2A2DB: malloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==750== by 0x4016A6: main (saf_logger.c:333) > ==750== > ==750== LEAK SUMMARY: > ==750== definitely lost: 0 bytes in 0 blocks > ==750== indirectly lost: 0 bytes in 0 blocks > ==750== possibly lost: 0 bytes in 0 blocks > ==750== still reachable: 99 bytes in 2 blocks > ==750== suppressed: 0 bytes in 0 blocks > > Best regards, > Giang Do > > On 01/09/2015 12:34, Mathivanan Naickan Palanivelu wrote: > > Hi Giang, > > > > Iam unable to apply the patch on the latest staging. > > However, the changes look straight forward. > > Should we also free logBuf? > > > > Thanks, > > Mathi. > > > >> -----Original Message----- > >> From: giang [mailto:[email protected]] > >> Sent: Tuesday, August 25, 2015 11:24 AM > >> To: Mathivanan Naickan Palanivelu; [email protected]; > >> [email protected] > >> Cc: [email protected] > >> Subject: [PATCH 1 of 1] log: saflogger does not check invalid > options > >> [#1367] > >> > >> osaf/tools/saflog/saflogger/saf_logger.c | 49 > >> ++++++++++++++++++++++--------- > >> 1 files changed, 35 insertions(+), 14 deletions(-) > >> > >> > >> If there are invalid non-options, show them, recommend use --help > and > >> then exit. > >> > >> diff --git a/osaf/tools/saflog/saflogger/saf_logger.c > >> b/osaf/tools/saflog/saflogger/saf_logger.c > >> --- a/osaf/tools/saflog/saflogger/saf_logger.c > >> +++ b/osaf/tools/saflog/saflogger/saf_logger.c > >> @@ -311,14 +311,48 @@ int main(int argc, char *argv[]) > >> logRecord.logHeader.genericHdr.logSeverity = > >> get_severity(optarg); > >> break; > >> case 'h': > >> + usage(); > >> + exit(EXIT_SUCCESS); > >> + break; > >> case '?': > >> default: > >> - usage(); > >> + fprintf(stderr, "Try saflogger -h for more > >> information.\n"); > >> exit(EXIT_FAILURE); > >> break; > >> } > >> } > >> > >> + if (optind >= argc) { > >> + /* No body of log record */ > >> + } > >> + else if (optind == argc - 1) { > >> + /* Create body of log record */ > >> + int sz; > >> + char *logBuf = NULL; > >> + sz = strlen(argv[optind]); > >> + logBuf = malloc(sz + 64); /* add space for index/id in > >> periodic writes */ > >> + strcpy(logBuf, argv[optind]); > >> + logBuffer.logBufSize = sz; > >> + logBuffer.logBuf = (SaUint8T *)logBuf; > >> + logRecord.logBuffer = &logBuffer; > >> + } > >> + else { > >> + fprintf(stderr, "Invalid argument.\n"); > >> + fprintf(stderr, "Enclose message in quotation marks \"\" e.g. > >> \""); > >> + while (optind < argc) > >> + { > >> + fprintf(stderr, "%s", argv[optind++]); > >> + if (optind < argc) > >> + fprintf(stderr, " "); > >> + else > >> + fprintf(stderr, "\"\n"); > >> + } > >> + fprintf(stderr, "Try saflogger -h for more information.\n"); > >> + exit(EXIT_FAILURE); > >> + } > >> + > >> + logStreamName.length = strlen((char *)logStreamName.value); > >> + > >> if (logRecord.logHdrType == SA_LOG_NTF_HEADER) { > >> /* Setup some valid values */ > >> logRecord.logHeader.ntfHdr.notificationId = > >> SA_NTF_IDENTIFIER_UNUSED; @@ -329,19 +363,6 @@ int main(int argc, > >> char > >> *argv[]) > >> logRecord.logHeader.ntfHdr.eventTime = > >> get_current_SaTime(); > >> } > >> > >> - logStreamName.length = strlen((char *)logStreamName.value); > >> - > >> - /* Create body of log record (if any) */ > >> - if (optind < argc) { > >> - int sz; > >> - char *logBuf = NULL; > >> - sz = strlen(argv[optind]); > >> - logBuf = malloc(sz + 64); /* add space for index/id in > >> periodic writes */ > >> - strcpy(logBuf, argv[optind]); > >> - logBuffer.logBufSize = sz; > >> - logBuffer.logBuf = (SaUint8T *)logBuf; > >> - logRecord.logBuffer = &logBuffer; > >> - } > >> > >> wait_time = 0; > >> error = saLogInitialize(&logHandle, &logCallbacks, > &logVersion); ------------------------------------------------------------------------------ Monitor Your Dynamic Infrastructure at Any Scale With Datadog! Get real-time metrics from all of your servers, apps and tools in one place. SourceForge users - Click here to start your Free Trial of Datadog now! http://pubads.g.doubleclick.net/gampad/clk?id=241902991&iu=/4140 _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
