[devel] [PATCH 1 of 1] CORE: use _exit instead of exit in daemon_exit #651
osaf/libs/core/common/daemon.c | 8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) use _exit instead of exit in daemon_exit diff --git a/osaf/libs/core/common/daemon.c b/osaf/libs/core/common/daemon.c --- a/osaf/libs/core/common/daemon.c +++ b/osaf/libs/core/common/daemon.c @@ -43,6 +43,8 @@ #define DEFAULT_RUNAS_USERNAME "opensaf" +extern void __gcov_flush(void) __attribute__((weak)); + static char __pidfile[NAME_MAX]; static char __tracefile[NAME_MAX]; static char __runas_username[UT_NAMESIZE]; @@ -364,7 +366,11 @@ static void sigterm_handler(int sig) void daemon_exit(void) { syslog(LOG_NOTICE, "exiting on signal %d", SIGTERM); - exit(0); + + if (__gcov_flush) { + __gcov_flush(); + } + _exit(0); } /** -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0 of 1] Review Request for #651
Summary: CORE: use _exit instead of exit daemon_exit Review request for Trac Ticket(s): #651 Peer Reviewer(s): hans.fe...@ericsson.com;anders.wid...@ericsson.com;ramesh.bet...@oracle.com Pull request to: Affected branch(es): default Development branch: Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries y Samples n Tests n Other n Comments (indicate scope for each "y" above): - <> changeset 72af35311c5089d436444f00402215bc5915e483 Author: Hans Nordeback Date: Mon, 09 Dec 2013 09:16:01 +0100 CORE: use _exit instead of exit in daemon_exit #651 use _exit instead of exit in daemon_exit Complete diffstat: -- osaf/libs/core/common/daemon.c | 8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) Testing Commands: - <> Testing, Expected Results: -- <> Conditions of Submission: - <> Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 0 of 3] Review Request for IMM: Implementation of NO_DANGLING flag [#49]
Additional comments. 9) In checkNoDanglingReferencesModify (to be renamed validateNoDanglingRefsModify): +LOG_NO("ERR_FAILED_OPERATION: NO_DANGLING reference (%s) does not exist", +av->getValueC_str()); Says the reference does not exist, but the reference exists, the problem is that it is dangling. Also add tghe ccbId to assist troubleshooing. So change to: +LOG_NO("ERR_FAILED_OPERATION: NO_DANGLING reference (%s) is dangling (Ccb %u)", +av->getValueC_str(), ccb->mId); -- +LOG_NO("ERR_FAILED_OPERATION: NO_DANGLING Reference (%s) " +"cannot be a non PRTO (Ccb %u)", Change to: +LOG_NO("ERR_FAILED_OPERATION: NO_DANGLING reference (%s) " +"refers to a non-persistent RTO (Ccb %u)", - +LOG_ER("ERR_FAILED_OPERATION: NO_DANGLING reference (%s) is " +"flagged for deleting by another CCB", +av->getValueC_str()); Change to: +LOG_ER("ERR_FAILED_OPERATION: NO_DANGLING reference (%s) " +"refers to object flagged for delete by another CCB: %u, (this Ccb %u)", +av->getValueC_str(), omi->second->mCcbId , ccb->mId); - +if((omi->second->mObjFlags & IMM_CREATE_LOCK) && (omi->second->mCcbId != ccb->mId)) { +TRACE_LEAVE(); +return SA_AIS_ERR_FAILED_OPERATION; // Create flag set by other CCB +} Should also have a log printout: +if((omi->second->mObjFlags & IMM_CREATE_LOCK) && (omi->second->mCcbId != ccb->mId)) { LOG_ER("ERR_FAILED_OPERATION: NO_DANGLING reference (%s) " "refers to object flagged for create by another CCB: %u, (this Ccb %u)", av->getValueC_str(), omi->second->mCcbId , ccb->mId); +TRACE_LEAVE(); +return SA_AIS_ERR_FAILED_OPERATION; +} -- 10) In checkNoDanglingReferencesDelete (to be renamed validateNoDanglingRefsDelete): +ObjectMap::iterator omi = sObjectMap.find(omit->first.c_str()); +if(omi == sObjectMap.end()) { +TRACE_LEAVE(); +return SA_AIS_OK; +} To me it seems that something is wrong here. How can there be a delete mutation in this ccb for an object that currently does not exist ? This will in fact assert later in commitDelete. May as well assert here osafassert(omi != sObjectMap.end()); - +if(!(ommi->second->mObjFlags & IMM_DELETE_LOCK)) { +LOG_ER("ERR_FAILED_OPERATION: Object with NO_DANGLING reference to the deleting object " + "is not flagged for the deletion"); +rc = SA_AIS_ERR_FAILED_OPERATION; Need to provide more information for troubleshooting. (I spontaneously thought we needed to check that the other object is being deleted by the *same* ccb. But I that has already been checked in the interference check at the operation stage.. +if(!(ommi->second->mObjFlags & IMM_DELETE_LOCK)) { +LOG_ER("ERR_FAILED_OPERATION: Delete of object %s would violate NO_DANGLING reference " "from object %s, not scheduled for delete by this CCB:%u",.); +rc = SA_AIS_ERR_FAILED_OPERATION; - +if(ommi->second->mCcbId != ccb->mId) { +LOG_ER("ERR_FAILED_OPERATION: Object with NO_DANGLING reference to the deleting object " + "is deleted by another CCB"); Has not this case already been already been checked/eliminated by the interference check at the operation stage, of this ccb and/or the other ccb ? If the check is redundant, then it should be made into an osafassert. If not, just use the same error log message as the previous one. The problem being that the other object is not scheduled for delete by this ccb. The fact that it happens to be scheduled for delete by another ccb is sort of not relevant to what is wrong with *this* ccb. --- Still a bit more comming. /AndersBj -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base.
Re: [devel] [PATCH 1 of 1] CORE: use _exit instead of exit in daemon_exit #651
Ack with comments. 1) In the commit message: * The component name should be base:, not CORE: * The ticket number should be surrounded with square brackets so that SourceForge recognizes it as a ticket reference. * The commit message should have a longer description explaining why this change was done (i.e. that we wish to dump coverage data on termination, but that exit() cannot be used since it is not thread-safe). 2) I usually prefer to use _Exit() over _exit() since the former one is declared as nothrow. regards, Anders Widell 2013-12-09 09:23, Hans Nordeback skrev: > osaf/libs/core/common/daemon.c | 8 +++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > > use _exit instead of exit in daemon_exit > > diff --git a/osaf/libs/core/common/daemon.c b/osaf/libs/core/common/daemon.c > --- a/osaf/libs/core/common/daemon.c > +++ b/osaf/libs/core/common/daemon.c > @@ -43,6 +43,8 @@ > > #define DEFAULT_RUNAS_USERNAME "opensaf" > > +extern void __gcov_flush(void) __attribute__((weak)); > + > static char __pidfile[NAME_MAX]; > static char __tracefile[NAME_MAX]; > static char __runas_username[UT_NAMESIZE]; > @@ -364,7 +366,11 @@ static void sigterm_handler(int sig) > void daemon_exit(void) > { > syslog(LOG_NOTICE, "exiting on signal %d", SIGTERM); > - exit(0); > + > + if (__gcov_flush) { > + __gcov_flush(); > + } > + _exit(0); > } > > /** -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] CORE: use _exit instead of exit in daemon_exit #651
Ack but please use ticket 581 instead. And please put in a reason for the change in the commit message body /HansF On 12/09/2013 09:23 AM, Hans Nordeback wrote: > osaf/libs/core/common/daemon.c | 8 +++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > > use _exit instead of exit in daemon_exit > > diff --git a/osaf/libs/core/common/daemon.c b/osaf/libs/core/common/daemon.c > --- a/osaf/libs/core/common/daemon.c > +++ b/osaf/libs/core/common/daemon.c > @@ -43,6 +43,8 @@ > > #define DEFAULT_RUNAS_USERNAME "opensaf" > > +extern void __gcov_flush(void) __attribute__((weak)); > + > static char __pidfile[NAME_MAX]; > static char __tracefile[NAME_MAX]; > static char __runas_username[UT_NAMESIZE]; > @@ -364,7 +366,11 @@ static void sigterm_handler(int sig) > void daemon_exit(void) > { > syslog(LOG_NOTICE, "exiting on signal %d", SIGTERM); > - exit(0); > + > + if (__gcov_flush) { > + __gcov_flush(); > + } > + _exit(0); > } > > /** > > -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1 of 1] build: Add extra GCC hardening compilation flags [#650]
00-README.conf | 2 +- Makefile.common | 4 ++-- README | 16 configure.ac | 18 ++ tools/cluster_sim_uml/README | 2 +- 5 files changed, 34 insertions(+), 8 deletions(-) By default, build with the extra hardening flags "-D_FORTIFY_SOURCE=2 -fstack-protector" for improved security and enhanced run-time error detection. The flags can be overridden by setting the environment variable OSAF_HARDEN_FLAGS when building OpenSAF. Note that -D_FORTIFY_SOURCE=2 is only enabled in optimized builds. To reduce the risk that a user accidentally builds without optimization by overriding the default CFLAGS and/or CXXFLAGS, the README files have been updated to recommend passing preprocessor definitions using CPPFLAGS instead of CFLAGS. diff --git a/00-README.conf b/00-README.conf --- a/00-README.conf +++ b/00-README.conf @@ -85,7 +85,7 @@ file does not have to be changed unless: - OpenSAF should run as a different UNIX group and user than the default 'opensaf' group/user. - If OpenSAF was built with the flags "CFLAGS=-DRUNASROOT", then + If OpenSAF was built with the flags "CPPFLAGS=-DRUNASROOT", then change OPENSAF_GROUP and OPENSAF_USER to root i.e. for old (<4.2) behaviour. For any other user, change OPENSAF_GROUP and OPENSAF_USER accordingly diff --git a/Makefile.common b/Makefile.common --- a/Makefile.common +++ b/Makefile.common @@ -12,8 +12,8 @@ AM_CPPFLAGS = \ $(CORE_INCLUDES) \ $(all_includes) -AM_CFLAGS = -Wall -fno-strict-aliasing -Werror -fPIC -AM_CXXFLAGS = -Wall -fno-strict-aliasing -Werror -fPIC -D__STDC_FORMAT_MACROS +AM_CFLAGS = -Wall -fno-strict-aliasing -Werror -fPIC @OSAF_HARDEN_FLAGS@ +AM_CXXFLAGS = -Wall -fno-strict-aliasing -Werror -fPIC -D__STDC_FORMAT_MACROS @OSAF_HARDEN_FLAGS@ AM_LDFLAGS = -ldl -lrt -lpthread # diff --git a/README b/README --- a/README +++ b/README @@ -293,7 +293,7 @@ 1.1.1, 1.1.2 etc. To re-enable the old (pre 4.3) non flat addressing, configure the constant MDS_USE_SUBSLOT_ID=1 at configure time as in: -% ./configure CFLAGS="-DMDS_USE_SUBSLOT_ID=1 ..." +% ./configure CPPFLAGS="-DMDS_USE_SUBSLOT_ID=1 ..." In the non flat scheme, the slot ID is shifted up 4 bits and subslot ID is added in the 4 LSB. The consequence of this is reduced number of @@ -308,7 +308,7 @@ 2) Run as root (optional) If the old (<4.2) behaviour of running all processes as root is desired, use the following configure command: -% ./configure CFLAGS=-DRUNASROOT +% ./configure CPPFLAGS=-DRUNASROOT 3) Configure TIPC importance (optional) @@ -317,13 +317,21 @@ The default TIPC importance is LOW for a In some cases the default importance must be changed if e.g. an application starves the LOW importance communication level. To change the default importance, use the following configure command - % ./configure CFLAGS=-DTIPCIMPORTANCE=level + % ./configure CPPFLAGS=-DTIPCIMPORTANCE=level where level is any of TIPC_LOW_IMPORTANCE, TIPC_MEDIUM_IMPORTANCE or TIPC_HIGH_IMPORTANCE - e.g. configure CFLAGS=-DTIPCIMPORTANCE=TIPC_HIGH_IMPORTANCE + e.g. configure CPPFLAGS=-DTIPCIMPORTANCE=TIPC_HIGH_IMPORTANCE Note: Giving same importance to AVND & all other Opensaf models is not preferred option. The behavior is unsupported. +4) Configure GCC hardening options (optional) + +By default, the options "-fstack-protector -D_FORTIFY_SOURCE=2" are passed to +GCC for improved security. You can override these options by setting the +OSAF_HARDEN_FLAGS when configuring OpenSAF. For example: + + % ./configure OSAF_HARDEN_FLAGS="-fstack-protector-all -D_FORTIFY_SOURCE=2" + If you are using a released archive (dist tarball) follow the simple common steps: diff --git a/configure.ac b/configure.ac --- a/configure.ac +++ b/configure.ac @@ -545,6 +545,22 @@ if test "$enable_imm_pbe" = yes; then PKG_CHECK_MODULES([SQLITE3], [sqlite3]) fi +if test -z "$OSAF_HARDEN_FLAGS"; then + # _FORTIFY_SOURCE requires optimization, so only enable it in optimized + # builds, i.e. when -O is present in both CFLAGS and CXXFLAGS. + if echo "${CFLAGS}" | grep -q -- -O; then + if echo "${CXXFLAGS}" | grep -q -- -O; then + OSAF_HARDEN_FLAGS="-D_FORTIFY_SOURCE=2" + fi + fi + # Also check for -O0 (which explicitly disables optimisation) + if echo "${CFLAGS} ${CXXFLAGS}" | grep -q -- -O0; then + OSAF_HARDEN_FLAGS="" + fi + OSAF_HARDEN_FLAGS="${OSAF_HARDEN_FLAGS} -fstack-protector" +fi +AC_SUBST(OSAF_HARDEN_FLAGS) + # # Checks for header files. # @@ -925,8 +941,10 @@ echo "" echo " Compiling Options:" echo "${ECHO_T} C Compiler: ${CC}" echo "${ECHO_T} C++ Compiler: ${CXX}" +echo "${ECHO_T} CPPFLAGS: ${CPPFLAGS} ${AM_CPPFLA
[devel] [PATCH 0 of 1] Review Request for build: Add extra GCC hardening compilation flags [#650]
Summary: build: Add extra GCC hardening compilation flags [#650] Review request for Trac Ticket(s): 650 Peer Reviewer(s): Mathi Pull request to: Affected branch(es): default(4.4) Development branch: default Impacted area Impact y/n Docsy Build systemy RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - changeset f627d356bf1737c254ac36b2d008c5b787093d21 Author: Anders Widell Date: Mon, 09 Dec 2013 12:38:54 +0100 build: Add extra GCC hardening compilation flags [#650] By default, build with the extra hardening flags "-D_FORTIFY_SOURCE=2 -fstack-protector" for improved security and enhanced run-time error detection. The flags can be overridden by setting the environment variable OSAF_HARDEN_FLAGS when building OpenSAF. Note that -D_FORTIFY_SOURCE=2 is only enabled in optimized builds. To reduce the risk that a user accidentally builds without optimization by overriding the default CFLAGS and/or CXXFLAGS, the README files have been updated to recommend passing preprocessor definitions using CPPFLAGS instead of CFLAGS. Complete diffstat: -- 00-README.conf | 2 +- Makefile.common | 4 ++-- README | 16 configure.ac | 18 ++ tools/cluster_sim_uml/README | 2 +- 5 files changed, 34 insertions(+), 8 deletions(-) Testing Commands: - Build OpenSAF in various ways, e.g. with default CFLAGS, and with CFLAGS that disable optimization. Testing, Expected Results: -- OpenSAF should build and start successfully. Conditions of Submission: - Ack from Mathi Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.hgrc file (i.e. username, email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad
Re: [devel] [PATCH 1 of 1] amfd: use saflog for CCB validation errors [#86]
Can I have some feedback? Right now we have no logging for CCB related validation errors. Thanks, Hans On 11/28/2013 10:24 AM, Hans Feldt wrote: > osaf/services/saf/amf/amfd/cluster.cc | 2 +- > osaf/services/saf/amf/amfd/imm.cc | 14 ++ > 2 files changed, 11 insertions(+), 5 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/cluster.cc > b/osaf/services/saf/amf/amfd/cluster.cc > --- a/osaf/services/saf/amf/amfd/cluster.cc > +++ b/osaf/services/saf/amf/amfd/cluster.cc > @@ -118,7 +118,7 @@ static SaAisErrorT ccb_completed_modify_ > SaTimeT cluster_startup_timeout = *((SaTimeT > *)attr_mod->modAttr.attrValues[0]); > if (0 == cluster_startup_timeout) { > report_ccb_validation_error(opdata, "Invalid > saAmfClusterStartupTimeout %llu", > - > avd_cluster->saAmfClusterStartupTimeout); > + cluster_startup_timeout); > rc = SA_AIS_ERR_BAD_OPERATION; > goto done; > } > diff --git a/osaf/services/saf/amf/amfd/imm.cc > b/osaf/services/saf/amf/amfd/imm.cc > --- a/osaf/services/saf/amf/amfd/imm.cc > +++ b/osaf/services/saf/amf/amfd/imm.cc > @@ -1792,12 +1792,18 @@ void report_ccb_validation_error(const C > va_start(ap, format); > (void) vsnprintf(err_str, sizeof(err_str), format, ap); > va_end(ap); > - TRACE("%s", err_str); > - if (opdata != NULL) > + > + if (opdata != NULL) { > + TRACE("%s", err_str); > + saflog(LOG_NOTICE, amfSvcUsrName, "CCB %llu validation error: > %s", > +opdata->ccbId, err_str); > (void) saImmOiCcbSetErrorString(avd_cb->immOiHandle, > opdata->ccbId, err_str); > - else > - LOG_WA("%s", err_str); > + } else { > + // errors found during initial configuration read > + LOG_WA("configuration validation error: %s", err_str); > + } > } > + > /** >* Respond admin op to IMM >* @param immOiHandle > > -- > Rapidly troubleshoot problems before they affect your business. Most IT > organizations don't have a clear picture of how application performance > affects their revenue. With AppDynamics, you get 100% visibility into your > Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! > http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk > ___ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] Additional patch for PLM compilation error, for patch-11 (#537)
If PLM is now the only user of this please change PLM instead. /Hans On 12/09/2013 08:31 AM, ramesh.bet...@oracle.com wrote: > osaf/libs/core/include/ncs_osprm.h | 8 > 1 files changed, 8 insertions(+), 0 deletions(-) > > > diff --git a/osaf/libs/core/include/ncs_osprm.h > b/osaf/libs/core/include/ncs_osprm.h > --- a/osaf/libs/core/include/ncs_osprm.h > +++ b/osaf/libs/core/include/ncs_osprm.h > @@ -798,6 +798,14 @@ uint32_t ncs_os_posix_mq(NCS_OS_POSIX_MQ > > / > #define m_NCS_OS_GET_TIME_STAMP(timestamp) timestamp=time((time_t*)0) > > +#ifndef m_GET_ASCII_DATE_TIME_STAMP > +#define m_GET_ASCII_DATE_TIME_STAMP(timestamp, asc_timestamp) \ > +{ \ > +timestamp=(time_t) (time((time_t *) 0)); \ > +strftime((char *)(asc_timestamp), 40, "%d%b%Y_%H.%M.%S", > localtime(×tamp)); \ > +} > +#endif > + > > > / >** > ** > > -- > Sponsored by Intel(R) XDK > Develop, test and display web and hybrid apps with a single code base. > Download it for free now! > http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk > ___ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 0 of 3] Review Request for IMM: Implementation of NO_DANGLING flag [#49]
Additional comments (third reply mail). 11) In ImmModel::addNoDanglingReferences(ObjectInfo *object) Comment should explain that this function extracts all NO_DANGLING references that exist in "object" and insert them into sReverseRefObjectMap. Thats with 'object' as source and the object matching the dn in the no-dangling attribute as target. This is otherwise a fairly simple and clean function. --- 12) In ImmModel::removeNoDanglingReferences(ObjectInfo *object, ObjectInfo *afim, bool removeAll) The leading comment must be improved. The function does not operate on the argument "object", it operates on sReverseRefObjectMMap. I think the 'removeAll' argument could probably be called removeRefsToObject ? Classic object-orientation would have made sense here but I am no insisting on that now. This function is not quite symetric with addNoDanglingReferences. So the comment should be changed say something like: /* * Delete from sReverseRefObjectMMap all NO_DANGLING reference values existing *in* 'object' . * If "removeRefsToObject" parameter is set to true, then *also* delete from sReverseRefObjectMMap all * NO_DANGLING references pointing *to* 'object.' . * "removeRefsToObject" parameter is set to "false" by default, and is an optional parameter. */ 13) ImmModel::addNoDanglingReferenceSet(ObjectInfo *obj, ObjectNameSet &dnSet) The function adds references to sReverseRefObjectMap. The leading comments could explain what references are constructed from the input arguments. In essence each reference is constructed from one element in the dnSet argument as *source* and with obj as *target*. But then it also filters on CREATE_LOCK on source/dnSet-element <-- Something needs to be said about that ccb-state-relation. Thus only references from sources that are currently objects being created (not yet committed) are inserted. - 14) ImmModel::removeNoDanglingReferenceSet(ObjectInfo *obj, ObjectNameSet &dnSet) Again, comment describing the function does not say much. Here is my understanding: The function removes references from sReverseRefObjectMap. For each dn in dnSet, it removes at most ONE reference in sReverseRefObjectMap, where the DN is the *target* and obj is the *source* Contrary (unsymmetrically) to the addNoDanglingReferenceSet, this delete function is not sensitive to any CCB locking state. -- 15) In ImmModel::commitCreate(ObjectInfo* obj) if(obj->mObjFlags & IMM_NO_DANGLING_FLAG) { /* First remove all created NO_DANGLING references in the operation phase, * than add all NO_DANGLING references. * Check for existence of objects is done before calling ccbCommit function. */ removeNoDanglingReferences(obj, obj); addNoDanglingReferences(obj); } The comment simply states what is obviously done in the next two statements. It should explain *why* it is doing the remove.. before the create..., which is not obvious. --- 16) In ImmModel::commitModify() This is where the add/remove NoDanglingReferenceSet are used. The comment: +/* Computation for which NO_DANGLING references will be removed, and which will be added */ Should say something like: The set of references in the before-image minus the set of references in the after-image constitutes the set of references *removed* by the (possibly chained) modify operation. The set of references in the after-image minus the set of references in the before-image constitutes the set of references *added* by the (posibly chained) modify operation. - 17) In mmModel::commitDelete(const std::string& dn) Unnecessary removal of reset of flag IMM_DELETE_ROOT. A statement is removed: if(oi->second->mObjFlags & IMM_DELETE_ROOT) { oi->second->mObjFlags &= ~IMM_DELETE_ROOT; <=== While not logically wrong, since the object is about to be deleted, I dont like to see "cleanup" in logic that has nothing to do with NO_DANGLING. Also the resetting of the delete-root flag is in the same spirit as zeroing members in a struct/class before a delete/free. Its not necessary, but its a low cost precaution, (reduction of risk) in case an illegal reference remains somewhere (i.e. a dangling hard pointer ... a bit OFF recursive irony). In this case I am paranoid about getting that delete root bit wrong anywhere because it affects the maintentance of the child-counter in the imm tree, which affects the correctness of all searches in the imm. Imagine if someone rearranges this code in the future... - N
Re: [devel] [PATCH 3 of 3] IMMTOOLS: Add support of NO_DANGLING flag to IMM tools [#49]
Hi Zoran, make rpm fails after applying this patch series. Entry needs to be added for saImmOm_A_2_13.h in opensaf.spec file. Regards, Sirisha On 12/05/2013 08:34 PM, Zoran Milinkovic wrote: > osaf/tools/safimm/immcfg/imm_import.cc | 2 ++ > osaf/tools/safimm/immdump/imm_xmlw_dump.cc | 9 + > osaf/tools/safimm/immlist/imm_list.c | 3 +++ > 3 files changed, 14 insertions(+), 0 deletions(-) > > > Support reference integrity (NO_DANGLING flag) to IMM tools > > diff --git a/osaf/tools/safimm/immcfg/imm_import.cc > b/osaf/tools/safimm/immcfg/imm_import.cc > --- a/osaf/tools/safimm/immcfg/imm_import.cc > +++ b/osaf/tools/safimm/immcfg/imm_import.cc > @@ -1832,6 +1832,8 @@ static SaImmAttrFlagsT charsToFlagsHelpe > return SA_IMM_ATTR_NOTIFY; > } else if (len == strlen("SA_NO_DUPLICATES")&& strncmp((const > char*)str, "SA_NO_DUPLICATES", len) == 0) { > return SA_IMM_ATTR_NO_DUPLICATES; > + } else if (len == strlen("SA_NO_DANGLING")&& strncmp((const char*)str, > "SA_NO_DANGLING", len) == 0) { > + return SA_IMM_ATTR_NO_DANGLING; > } > > /* strlen("SA_NO_DUPLICATES") == 16 (the longest flag name) > diff --git a/osaf/tools/safimm/immdump/imm_xmlw_dump.cc > b/osaf/tools/safimm/immdump/imm_xmlw_dump.cc > --- a/osaf/tools/safimm/immdump/imm_xmlw_dump.cc > +++ b/osaf/tools/safimm/immdump/imm_xmlw_dump.cc > @@ -508,6 +508,15 @@ void flagsToXMLw(SaImmAttrDefinitionT_2* > } > } > > +if (flags& SA_IMM_ATTR_NO_DANGLING) > +{ > +if(xmlTextWriterWriteElement(writer, (xmlChar*) "flag", > + (xmlChar*) "SA_NO_DANGLING")< 0 ) { > +std::cout<< "Error at xmlTextWriterWriteElement (flag - > SA_NO_DANGLING)"<< std::endl; > +exit(1); > +} > +} > + > } > > void typeToXMLw(SaImmAttrDefinitionT_2* p, xmlTextWriterPtr writer) > diff --git a/osaf/tools/safimm/immlist/imm_list.c > b/osaf/tools/safimm/immlist/imm_list.c > --- a/osaf/tools/safimm/immlist/imm_list.c > +++ b/osaf/tools/safimm/immlist/imm_list.c > @@ -323,6 +323,9 @@ static void display_class_definition(con > if (attrDefinition->attrFlags& > SA_IMM_ATTR_NO_DUPLICATES) > printf(", NO_DUPLICATES"); > > + if (attrDefinition->attrFlags& SA_IMM_ATTR_NO_DANGLING) > + printf(", NO_DANGLING"); > + > } else if (attrDefinition->attrFlags& SA_IMM_ATTR_RUNTIME) { > if (attrDefinition->attrDefaultValue != NULL) { > printf(" = "); > > -- > Sponsored by Intel(R) XDK > Develop, test and display web and hybrid apps with a single code base. > Download it for free now! > http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk > ___ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel -- Sponsored by Intel(R) XDK Develop, test and display web and hybrid apps with a single code base. Download it for free now! http://pubads.g.doubleclick.net/gampad/clk?id=111408631&iu=/4140/ostg.clktrk ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel