Hi Ack, with comments
*** Constants should be named according to google style guide. Example: const char cmd[] = "immcfg -a logRecordDestinationConfiguration"; should be - const char kCmd[] = "immcfg -a logRecordDestinationConfiguration"; *** Better more descriptive names should be given to constants, variables etc. An example is cmd that does not say very much about what command it represents. It is also confusing that another constant "cmdName" used in a similar way also exist. Besides " cmdName" is also misleading in itself since its content is in no way a "name" and it is also not only a command. Could be named something like kSetDestinationConfiguration or maybe kSetDestConf In general think of the reader. Try to make the code easy to read. Do not "hide" anything behind cryptic names that others has to analyze before being able to understand the code Thanks Lennart > -----Original Message----- > From: Canh Van Truong [mailto:[email protected]] > Sent: den 3 mars 2017 04:05 > To: Lennart Lund <[email protected]>; Vu Minh Nguyen > <[email protected]>; [email protected] > Cc: [email protected] > Subject: [PATCH 1 of 1] log: fix the problem cause the logtest 5 17 failed > [#2328] > > src/log/apitest/tet_cfg_destination.c | 249 ++++++++++++++++++----------- > ---- > 1 files changed, 136 insertions(+), 113 deletions(-) > > > Some test cases in test suite 18 has not cleared > logRecordDestinationConfiguratin > attribute that causes the test case logtest 5 17 fail after running test suite > logtest 18. > > This patch clear the attribute > logRecordDestinationConfiguration/saLogRecordDestination > to make empty after testing suite 18 > > diff --git a/src/log/apitest/tet_cfg_destination.c > b/src/log/apitest/tet_cfg_destination.c > --- a/src/log/apitest/tet_cfg_destination.c > +++ b/src/log/apitest/tet_cfg_destination.c > @@ -52,90 +52,91 @@ const char cmd[] = "immcfg -a logRecordD > // Verify it is OK to set an valid destination. > void cfgOneValidDest(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s", cmd, validDest, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - } > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is OK to set multi valid destinations. > void cfgMultiValidDest(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s", cmd, multiDest1, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > // Multiple configuration > sprintf(command,"%s+=\"%s\" %s", cmd, multiDest2, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - } > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is OK to delete one destination. > void delOneDest(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s", cmd, multiDest1, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > // Add multiple configuration > sprintf(command,"%s+=\"%s\" %s", cmd, multiDest2, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > // Delete one configuration > sprintf(command,"%s-=\"%s\" %s", cmd, multiDest2, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - } > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is OK to delete destinations > void delCfgDest(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s= %s", cmd, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - } > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is NOK to set type different from "unix" > void invalidTypeDestFn(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s 2> /dev/null", > @@ -148,7 +149,7 @@ void invalidTypeDestFn(void) > // Note: right format is one that have at least "name" and "type" > void invalidFmtDestFn(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s 2> /dev/null", > @@ -160,7 +161,7 @@ void invalidFmtDestFn(void) > // Verify it is NOK to set configuration with name contain special character > void invalidNameDestFn(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s 2> /dev/null", > @@ -173,20 +174,24 @@ void invalidNameDestFn(void) > // The rule is same "name" must go with same "value", and vice versa. > void duplicatedDestFn(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s", cmd, multiDest1, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > sprintf(command,"%s+=\"%s\" %s 2> /dev/null", > cmd, invalidDuplicatedDest, cfgObjDn); > rc = system(command); > + > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > + > rc_validate(WEXITSTATUS(rc), 1); > } > > @@ -206,48 +211,52 @@ const char cmdName[] = "immcfg -a saLogR > // Verify it is OK to set an valid destination name > void cfgOneDestName(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s", cmdName, validName, > systemDN); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - } > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmdName, systemDN); > + systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is OK to set multiple destination names > void cfgMultiDestName(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s=\"%s\" %s", cmdName, validName, > systemDN); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > sprintf(command,"%s+=\"%s\" %s", cmdName, multiName, > systemDN); > - rc = system(command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmdName, systemDN); > + systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is OK to clear destination name > void delDestName(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > > sprintf(command,"%s= %s", cmdName, systemDN); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - } > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + > + rc_validate(rc, 0); > } > > // Verify it is NOK to set an invalid destination name > @@ -288,7 +297,7 @@ bool is_executed_on_active_node() > // Verify if the record comes to destination or not > void writeToDest(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > bool disable_stdout = true; > SaConstStringT s_stdout = "1> /dev/null"; > @@ -303,20 +312,18 @@ void writeToDest(void) > > // 1) Configure an valid destination > sprintf(command,"%s=\"%s\" %s", cmd, validDest, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > // 2) Configure an valid destination name > sprintf(command,"%s=\"%s\" %s", cmdName, validName, > systemDN); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // Avoid getting old msg > @@ -324,11 +331,10 @@ void writeToDest(void) > > // 3) Send an log record to system log stream > sprintf(command,"%s \"%s_%d\"", sendCmd, sendMsg, r); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // 5) Verify if that sent msg comes to the end > @@ -346,7 +352,16 @@ tryagain: > fprintf(stderr, "Failed to perform cmd %s\n", > command); > } > } > + > rc_validate(WEXITSTATUS(rc), 0); > + > +clear_attr: > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > + > + sprintf(command,"%s='' %s", cmdName, systemDN); > + systemCall(command); > } > > // Verify if the record comes to destination or not > @@ -354,7 +369,7 @@ tryagain: > const char sendMsgNoDest[] = "[No dest name set] writing a record to > destination"; > void writeToNoDestName(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > bool disable_stdout = true; > SaConstStringT s_stdout = "1> /dev/null"; > @@ -369,20 +384,18 @@ void writeToNoDestName(void) > > // 1) Configure an valid destination > sprintf(command,"%s=\"%s\" %s", cmd, validDest, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > // 2) Delete Destination name > sprintf(command,"%s= %s", cmdName, systemDN); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // Avoid getting old msg > @@ -390,11 +403,10 @@ void writeToNoDestName(void) > > // 3) Send an log record to system log stream > sprintf(command,"%s \"%s_%d\"", sendCmd, sendMsg, r); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // 4) Sleep for a while (5s) as mds log server could busy > @@ -408,7 +420,13 @@ void writeToNoDestName(void) > if (WEXITSTATUS(rc) == 0) { > fprintf(stderr, "log record is written to local file\n"); > } > + > rc_validate(WEXITSTATUS(rc), 1); > + > +clear_attr: > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > } > > // Verify if the record comes to destination or not > @@ -416,7 +434,7 @@ void writeToNoDestName(void) > const char sendMsgNil[] = "[Dest name with nil configuration] writing a > record to destination"; > void writeToNilDestCfg(void) > { > - SaAisErrorT rc; > + int rc; > char command[1000]; > bool disable_stdout = true; > SaConstStringT s_stdout = "1> /dev/null"; > @@ -431,29 +449,26 @@ void writeToNilDestCfg(void) > > // 1) Configure an valid destination > sprintf(command,"%s=\"%s\" %s", cmd, validDest, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > return; > } > > // 2) Add a nil destination > sprintf(command,"%s+=\"%s\" %s", cmd, nildest, cfgObjDn); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // 3) Add destination name with nil destination configuation. > sprintf(command,"%s=\"%s\" %s", cmdName, nilname, systemDN); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // Avoid getting old msg > @@ -461,11 +476,10 @@ void writeToNilDestCfg(void) > > // 4) Send an log record to system log stream > sprintf(command,"%s \"%s_%d\"", sendCmd, sendMsg, r); > - rc = system(command); > - if (WEXITSTATUS(rc) != 0) { > - fprintf(stderr, "Failed to perform cmd %s\n", command); > - rc_validate(WEXITSTATUS(rc), 0); > - return; > + rc = systemCall(command); > + if (rc != 0) { > + rc_validate(rc, 0); > + goto clear_attr; > } > > // 4) Sleep for a while (2s) as mds log server could busy > @@ -479,7 +493,16 @@ void writeToNilDestCfg(void) > if (WEXITSTATUS(rc) == 0) { > fprintf(stderr, "log record is written to local file\n"); > } > + > rc_validate(WEXITSTATUS(rc), 1); > + > +clear_attr: > + // Cleanup by removing all values > + sprintf(command,"%s='' %s", cmd, cfgObjDn); > + systemCall(command); > + > + sprintf(command,"%s='' %s", cmdName, systemDN); > + systemCall(command); > } > > __attribute__ ((constructor)) static void cfgDest_constructor(void) ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
