Hi Mathi, Yes it is a fault that can be found on all branches but since this is an old defect that has not been a problem so far (the user actually has to do something wrong for the problem to occur). The ticket was originally written as a defect ticket but I changed it to enhancement. The reason is that the fix is not important enough to motivate the "risk" of adding new code to old branches. If you still think it should be a defect and be applied to all branches I don't have problem with that. Also even if the ticket remains an enhancement ticket it should maybe be applied to the 4.7 branch and the devel branch?
Otherwise: Ack with minor comments: 1. I got a Fuzz with the tet_LogOiOps.c file when applying the patch 2. In the commit message the component name shall be log: not logsv: I can fix both comments and push unless Mathi still wants this fix on all branches (defect). /Lennart > -----Original Message----- > From: Mathivanan Naickan Palanivelu [mailto:[email protected]] > Sent: den 30 september 2015 11:28 > To: Lennart Lund; Giang Do T; Vu Nguyen M > Cc: [email protected] > Subject: RE: [PATCH 1 of 1] logsv: crashed when performing admin op on > configurable obj class [#1420] > > ACK, tested. > I think this is a defect ticket. > Mathi. > > > -----Original Message----- > > From: Vu Minh Nguyen [mailto:[email protected]] > > Sent: Tuesday, September 29, 2015 7:55 AM > > To: Mathivanan Naickan Palanivelu; [email protected]; > > [email protected] > > Cc: [email protected] > > Subject: [PATCH 1 of 1] logsv: crashed when performing admin op on > > configurable obj class [#1420] > > > > osaf/services/saf/logsv/lgs/lgs_imm.c | 8 +++++++ > > tests/logsv/tet_LogOiOps.c | 39 > > +++++++++++++++++++++++++++++++++++ > > 2 files changed, 47 insertions(+), 0 deletions(-) > > > > > > logsv did not check if the operation is perfomed on configuration class or > > runtime one. Therefore, once the input valid, logsv did update the > attribute > > and though that was runtime one. So, it caused the trouble. > > > > Add the check will fix this issue. > > > > diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.c > > b/osaf/services/saf/logsv/lgs/lgs_imm.c > > --- a/osaf/services/saf/logsv/lgs/lgs_imm.c > > +++ b/osaf/services/saf/logsv/lgs/lgs_imm.c > > @@ -384,6 +384,14 @@ static void adminOperationCallback(SaImm > > > > if (opId == SA_LOG_ADMIN_CHANGE_FILTER) { > > /* Only allowed to update runtime objects > (application > > streams) */ > > + SaImmClassNameT className = > > immutil_get_className(objectName); > > + > > + if (!strcmp(className, "SaLogStreamConfig")) { > > + > > (void)immutil_saImmOiAdminOperationResult(immOiHandle, > > + > invocation, > > SA_AIS_ERR_NOT_SUPPORTED); > > + goto done; > > + } > > + > > if (stream->streamType != > STREAM_TYPE_APPLICATION) { > > report_om_error(immOiHandle, > invocation, > > > "Admin op change filter for non app > > stream"); diff --git a/tests/logsv/tet_LogOiOps.c > > b/tests/logsv/tet_LogOiOps.c > > --- a/tests/logsv/tet_LogOiOps.c > > +++ b/tests/logsv/tet_LogOiOps.c > > @@ -3263,6 +3263,44 @@ void verCCBWithInvalidValues(void) > > } > > } > > > > +/** > > + * Add test case for ticket #1420 > > + * logsv crashed if performing admin op on configurable obj class. > > + * Test steps: > > + * 1. Create an configurable obj class for app stream. > > + * 2. Perform admin op - changing saLogStreamSeverityFilter with valid > > value. > > + * 3. Verify that it fails to perform that op. > > + * 4. Delete previous created class. > > + */ > > +void verAdminOpOnConfClass(void) > > +{ > > + int rc; > > + char command[256]; > > + > > + /* Create an configurable obj class */ > > + strcpy(command, "immcfg -c SaLogStreamConfig > > safLgStrCfg=adminOp,safApp=safLogService " > > + "-a saLogStreamFileName=adminOp -a > > saLogStreamPathName=. -a saLogStreamMaxFilesRotated=4 " > > + "2> /dev/null"); > > + rc = system(command); > > + > > + if (WEXITSTATUS(rc) == 1) { > > + /* Failed to perfom command. Report test failed > and return. > > */ > > + fprintf(stderr, "Failed to perform command = %s > \n", > > command); > > + rc_validate(WEXITSTATUS(rc), 0); > > + return; > > + } > > + > > + /* Perfom admin operation on the configurable obj class */ > > + strcpy(command, "immadm -o 1 -p > > saLogStreamSeverityFilter:SA_UINT32_T:100 " > > + "safLgStrCfg=adminOp,safApp=safLogService > 2> > > /dev/null"); > > + rc = system(command); > > + rc_validate(WEXITSTATUS(rc), 1); > > + > > + /* Delete the created class */ > > + strcpy(command, "immcfg -d > > safLgStrCfg=adminOp,safApp=safLogService"); > > + rc = system(command); > > +} > > + > > __attribute__ ((constructor)) static void saOiOperations_constructor(void) > { > > /* Stream objects */ > > @@ -3383,6 +3421,7 @@ void verCCBWithInvalidValues(void) > > test_case_add(6, saLogOi_77, "Create: > saLogStreamMaxFilesRotated > > > 128, ERR"); > > test_case_add(6, saLogOi_78, "Create: > saLogStreamMaxFilesRotated > > == 128, ERR"); > > test_case_add(6, verMaxFilesRotated, "Create: > > saLogStreamMaxFilesRotated = 0, ERR"); > > + test_case_add(6, verAdminOpOnConfClass, "Perform admin > op on > > +configurable obj class, ERR"); > > > > /* Tests for modify */ > > test_case_add(6, saLogOi_100, "Modify: > saLogStreamSeverityFilter < > > 0x7f, Ok"); ------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
