Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
I agree with this. Assertions should be enabled also in the production code. My main argument is that when a bug has been detected (or should have been detected, if assertions had been enabled), then it is in most situations better (safer) to terminate the process rather than to continue executing in an unknown and presumably faulty state. I can recommend a good article about assertions, where they mention an example of a situation when it could be better to temporarily disable assertions: http://blog.regehr.org/archives/1091 In the Section "Are Assertions Enabled in Production Code?", they write: "On the other hand, I heard from a NASA flight software engineer that some of the tricky Mars landings have been done with assertions turned off because an assertion violation would have resulted in a system reboot and by the time the reboot had completed, the spacecraft would have hit the planet." I suppose that on a mission to Mars, it could make sense to temporarily disable assertions during the critical landing phase. But during space flight, it would be better to have the assertions enabled, so that the space probe recovers from errors. If you don't restart on a failed assertion, the software could be stuck forever in a faulty state and you may end up with a dead probe that you cannot communicate with. regards, Anders Widell 2014-04-07 09:38, Anders Björnerstedt skrev: > The idea of using asserts and compiling away asserts in production code is > quite silly in my opinion. > > To work with asserts in that way you need 100% code coverage in > function/component test, which is > impossible (i.e. impossibly expensive) to achieve as soon as the code becomes > moderatley complex. > The 100 % coverage driver is also sensitive to the slightest changes in that > code. So the idea > Could work for code that never changes. But then you could just as well > permanently remove the asserts. > > So get used to the idea that this idea never works in practice and the only > usefull way to use > asserts is to keep them in the production code. And what would be the problem > with that ? Performance ? > The typical assert only evaluates a local condition using simple primitives. > But even if there are some unusual or expensive checks on occasion in some > assert, if these checks are > judged necessary, so be it. Performance is tackled by profiling, removing > bottlenecks and reducing > Hot spots (if possible). If the hot-spot is in an assert then you can try to > optimize that assert just > as you would optimize any code. > > /AndersBj > > > > > -Original Message- > From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] > Sent: den 4 april 2014 16:52 > To: Anders Widell; Nagendra Kumar; Hans Feldt; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace > patricia tree db V2 [#713] > > validations and asserts are not the same thing. Asserts normally logs a > message and aborts and may be compiled away. /BR HansN > > -Original Message- > From: Anders Widell > Sent: den 4 april 2014 16:39 > To: Hans Nordebäck; Nagendra Kumar; Hans Feldt; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace > patricia tree db V2 [#713] > > Asserts are fine even if the input is supposed to already be validated. > But as I said, asserting that a pointer is not NULL is not so useful since > you crash anyway when dereferencing it. Possibly it can be useful as a way do > document the code, to say that you know this pointer is not NULL. Both humans > and e.g. static analysis tools can make use of such information. > > regards, > Anders Widell > > 2014-04-04 16:33, Hans Nordebäck skrev: >> "external" API normally needs to validate its input and further >> calling "internal" function these validations are not needed. Asserts can be >> used to detect bugs and are normally compiled away in release builds. >> I can add osafassert in these functions, but shouldn't the input >> already have been validated in calling functions? /BR HansN >> >> -----Original Message----- >> From: Anders Widell >> Sent: den 4 april 2014 16:27 >> To: Nagendra Kumar; Hans Nordebäck; Hans Feldt; Praveen Malviya >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to >> replace patricia tree db V2 [#713] >> >> Just some thoughts regarding NULL pointers: >> >> When I write a function, I by default assume that pointers passed to it as >> arguments are not NULL. Only if the API docu
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
The idea of using asserts and compiling away asserts in production code is quite silly in my opinion. To work with asserts in that way you need 100% code coverage in function/component test, which is impossible (i.e. impossibly expensive) to achieve as soon as the code becomes moderatley complex. The 100 % coverage driver is also sensitive to the slightest changes in that code. So the idea Could work for code that never changes. But then you could just as well permanently remove the asserts. So get used to the idea that this idea never works in practice and the only usefull way to use asserts is to keep them in the production code. And what would be the problem with that ? Performance ? The typical assert only evaluates a local condition using simple primitives. But even if there are some unusual or expensive checks on occasion in some assert, if these checks are judged necessary, so be it. Performance is tackled by profiling, removing bottlenecks and reducing Hot spots (if possible). If the hot-spot is in an assert then you can try to optimize that assert just as you would optimize any code. /AndersBj -Original Message- From: Hans Nordebäck [mailto:hans.nordeb...@ericsson.com] Sent: den 4 april 2014 16:52 To: Anders Widell; Nagendra Kumar; Hans Feldt; Praveen Malviya Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] validations and asserts are not the same thing. Asserts normally logs a message and aborts and may be compiled away. /BR HansN -Original Message- From: Anders Widell Sent: den 4 april 2014 16:39 To: Hans Nordebäck; Nagendra Kumar; Hans Feldt; Praveen Malviya Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] Asserts are fine even if the input is supposed to already be validated. But as I said, asserting that a pointer is not NULL is not so useful since you crash anyway when dereferencing it. Possibly it can be useful as a way do document the code, to say that you know this pointer is not NULL. Both humans and e.g. static analysis tools can make use of such information. regards, Anders Widell 2014-04-04 16:33, Hans Nordebäck skrev: > "external" API normally needs to validate its input and further > calling "internal" function these validations are not needed. Asserts can be > used to detect bugs and are normally compiled away in release builds. > I can add osafassert in these functions, but shouldn't the input > already have been validated in calling functions? /BR HansN > > -Original Message- > From: Anders Widell > Sent: den 4 april 2014 16:27 > To: Nagendra Kumar; Hans Nordebäck; Hans Feldt; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to > replace patricia tree db V2 [#713] > > Just some thoughts regarding NULL pointers: > > When I write a function, I by default assume that pointers passed to it as > arguments are not NULL. Only if the API documentation explicitly states that > a NULL pointer is a valid input for that argument, then the function needs to > handle it. Otherwise it is fine to crash (or assert) when receiving a NULL > pointer. An assert is typically not needed, since the program will crash when > dereferencing a NULL pointer anyway. > > Sometimes it is useful to allow function arguments to be NULL pointers. > One typical example is the C library functions free() and realloc(), where it > can be very handy to be able to pass a NULL pointer in case no memory has > been allocated (yet). The API documentation here clearly says what happens > when you pass a NULL pointer. > > regards, > Anders Widell > > 2014-04-04 15:04, Nagendra Kumar skrev: >> Ack with comment the functions added are not NULL safe. It crashed if passed >> NULL. Do we want to have some checks ? >> >> Thanks >> -Nagu >> >>> -Original Message- >>> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >>> Sent: 26 March 2014 16:58 >>> To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: [PATCH 1 of 1] amfd: use template class db to replace >>> patricia tree db >>> V2 [#713] >>> >>>osaf/services/saf/amf/amfd/include/db_template.h | 63 >>> >>>1 files changed, 63 insertions(+), 0 deletions(-) >>> >>> >>> diff --git a/osaf/services/saf/amf/amfd/include/db_template.h >>> b/osaf/services/saf/amf/amfd/include/db_template.h >>> new file mode 100644 >>> --- /dev/null &g
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Yes that is the contract thing in defensive programming, should be assert not osafassert Skickat från min Sony Xperia™-smartphone Anders Widell skrev Asserts are fine even if the input is supposed to already be validated. But as I said, asserting that a pointer is not NULL is not so useful since you crash anyway when dereferencing it. Possibly it can be useful as a way do document the code, to say that you know this pointer is not NULL. Both humans and e.g. static analysis tools can make use of such information. regards, Anders Widell 2014-04-04 16:33, Hans Nordebäck skrev: > "external" API normally needs to validate its input and further calling > "internal" function these validations are not needed. Asserts can be used to > detect bugs and are normally compiled away in release builds. > I can add osafassert in these functions, but shouldn't the input already have > been validated in calling functions? /BR HansN > > -Original Message- > From: Anders Widell > Sent: den 4 april 2014 16:27 > To: Nagendra Kumar; Hans Nordebäck; Hans Feldt; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace > patricia tree db V2 [#713] > > Just some thoughts regarding NULL pointers: > > When I write a function, I by default assume that pointers passed to it as > arguments are not NULL. Only if the API documentation explicitly states that > a NULL pointer is a valid input for that argument, then the function needs to > handle it. Otherwise it is fine to crash (or assert) when receiving a NULL > pointer. An assert is typically not needed, since the program will crash when > dereferencing a NULL pointer anyway. > > Sometimes it is useful to allow function arguments to be NULL pointers. > One typical example is the C library functions free() and realloc(), where it > can be very handy to be able to pass a NULL pointer in case no memory has > been allocated (yet). The API documentation here clearly says what happens > when you pass a NULL pointer. > > regards, > Anders Widell > > 2014-04-04 15:04, Nagendra Kumar skrev: >> Ack with comment the functions added are not NULL safe. It crashed if passed >> NULL. Do we want to have some checks ? >> >> Thanks >> -Nagu >> >>> -Original Message- >>> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >>> Sent: 26 March 2014 16:58 >>> To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: [PATCH 1 of 1] amfd: use template class db to replace >>> patricia tree db >>> V2 [#713] >>> >>>osaf/services/saf/amf/amfd/include/db_template.h | 63 >>> >>>1 files changed, 63 insertions(+), 0 deletions(-) >>> >>> >>> diff --git a/osaf/services/saf/amf/amfd/include/db_template.h >>> b/osaf/services/saf/amf/amfd/include/db_template.h >>> new file mode 100644 >>> --- /dev/null >>> +++ b/osaf/services/saf/amf/amfd/include/db_template.h >>> @@ -0,0 +1,63 @@ >>> +/* -*- OpenSAF -*- >>> + * >>> + * (C) Copyright 2014 The OpenSAF Foundation >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> +but >>> + * WITHOUT ANY WARRANTY; without even the implied warranty of >>> MERCHANTABILITY >>> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are >>> +licensed >>> + * under the GNU Lesser General Public License Version 2.1, February 1999. >>> + * The complete license can be accessed from the following location: >>> + * http://opensource.org/licenses/lgpl-license.php >>> + * See the Copying file included with the OpenSAF distribution for >>> +full >>> + * licensing terms. >>> + * >>> + * Author(s): Ericsson AB >>> + * >>> + */ >>> +#ifndef DB_TEMPLATE_H >>> +#defineDB_TEMPLATE_H >>> + >>> +#include >>> +#include >>> + >>> +template >>> +class AmfDb { >>> + public: >>> + void insert(T *obj); >>> + void erase(T *obj); >>> + T *find(const SaNameT *name); >>> + >>> + typedef std::map AmfDbMap; >>> + typedef typename AmfDbMap::const_iterator const_iterator; >>> + >>> + const_iterator begin() const {return db.begin();} >>> + const_iterator end() const {return db.end();} >>> + >>> + private: >>&
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
validations and asserts are not the same thing. Asserts normally logs a message and aborts and may be compiled away. /BR HansN -Original Message- From: Anders Widell Sent: den 4 april 2014 16:39 To: Hans Nordebäck; Nagendra Kumar; Hans Feldt; Praveen Malviya Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] Asserts are fine even if the input is supposed to already be validated. But as I said, asserting that a pointer is not NULL is not so useful since you crash anyway when dereferencing it. Possibly it can be useful as a way do document the code, to say that you know this pointer is not NULL. Both humans and e.g. static analysis tools can make use of such information. regards, Anders Widell 2014-04-04 16:33, Hans Nordebäck skrev: > "external" API normally needs to validate its input and further > calling "internal" function these validations are not needed. Asserts can be > used to detect bugs and are normally compiled away in release builds. > I can add osafassert in these functions, but shouldn't the input > already have been validated in calling functions? /BR HansN > > -Original Message- > From: Anders Widell > Sent: den 4 april 2014 16:27 > To: Nagendra Kumar; Hans Nordebäck; Hans Feldt; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to > replace patricia tree db V2 [#713] > > Just some thoughts regarding NULL pointers: > > When I write a function, I by default assume that pointers passed to it as > arguments are not NULL. Only if the API documentation explicitly states that > a NULL pointer is a valid input for that argument, then the function needs to > handle it. Otherwise it is fine to crash (or assert) when receiving a NULL > pointer. An assert is typically not needed, since the program will crash when > dereferencing a NULL pointer anyway. > > Sometimes it is useful to allow function arguments to be NULL pointers. > One typical example is the C library functions free() and realloc(), where it > can be very handy to be able to pass a NULL pointer in case no memory has > been allocated (yet). The API documentation here clearly says what happens > when you pass a NULL pointer. > > regards, > Anders Widell > > 2014-04-04 15:04, Nagendra Kumar skrev: >> Ack with comment the functions added are not NULL safe. It crashed if passed >> NULL. Do we want to have some checks ? >> >> Thanks >> -Nagu >> >>> -Original Message- >>> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >>> Sent: 26 March 2014 16:58 >>> To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: [PATCH 1 of 1] amfd: use template class db to replace >>> patricia tree db >>> V2 [#713] >>> >>>osaf/services/saf/amf/amfd/include/db_template.h | 63 >>> >>>1 files changed, 63 insertions(+), 0 deletions(-) >>> >>> >>> diff --git a/osaf/services/saf/amf/amfd/include/db_template.h >>> b/osaf/services/saf/amf/amfd/include/db_template.h >>> new file mode 100644 >>> --- /dev/null >>> +++ b/osaf/services/saf/amf/amfd/include/db_template.h >>> @@ -0,0 +1,63 @@ >>> +/* -*- OpenSAF -*- >>> + * >>> + * (C) Copyright 2014 The OpenSAF Foundation >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> +but >>> + * WITHOUT ANY WARRANTY; without even the implied warranty of >>> MERCHANTABILITY >>> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are >>> +licensed >>> + * under the GNU Lesser General Public License Version 2.1, February 1999. >>> + * The complete license can be accessed from the following location: >>> + * http://opensource.org/licenses/lgpl-license.php >>> + * See the Copying file included with the OpenSAF distribution for >>> +full >>> + * licensing terms. >>> + * >>> + * Author(s): Ericsson AB >>> + * >>> + */ >>> +#ifndef DB_TEMPLATE_H >>> +#defineDB_TEMPLATE_H >>> + >>> +#include >>> +#include >>> + >>> +template >>> +class AmfDb { >>> + public: >>> + void insert(T *obj); >>> + void erase(T *obj); >>> + T *find(const SaNameT *name); >>> + >>> + typedef std::map AmfDbMap; >>> + typedef typename AmfDbMap::const
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Asserts are fine even if the input is supposed to already be validated. But as I said, asserting that a pointer is not NULL is not so useful since you crash anyway when dereferencing it. Possibly it can be useful as a way do document the code, to say that you know this pointer is not NULL. Both humans and e.g. static analysis tools can make use of such information. regards, Anders Widell 2014-04-04 16:33, Hans Nordebäck skrev: > "external" API normally needs to validate its input and further calling > "internal" function these validations are not needed. Asserts can be used to > detect bugs and are normally compiled away in release builds. > I can add osafassert in these functions, but shouldn't the input already have > been validated in calling functions? /BR HansN > > -Original Message- > From: Anders Widell > Sent: den 4 april 2014 16:27 > To: Nagendra Kumar; Hans Nordebäck; Hans Feldt; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace > patricia tree db V2 [#713] > > Just some thoughts regarding NULL pointers: > > When I write a function, I by default assume that pointers passed to it as > arguments are not NULL. Only if the API documentation explicitly states that > a NULL pointer is a valid input for that argument, then the function needs to > handle it. Otherwise it is fine to crash (or assert) when receiving a NULL > pointer. An assert is typically not needed, since the program will crash when > dereferencing a NULL pointer anyway. > > Sometimes it is useful to allow function arguments to be NULL pointers. > One typical example is the C library functions free() and realloc(), where it > can be very handy to be able to pass a NULL pointer in case no memory has > been allocated (yet). The API documentation here clearly says what happens > when you pass a NULL pointer. > > regards, > Anders Widell > > 2014-04-04 15:04, Nagendra Kumar skrev: >> Ack with comment the functions added are not NULL safe. It crashed if passed >> NULL. Do we want to have some checks ? >> >> Thanks >> -Nagu >> >>> -Original Message- >>> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >>> Sent: 26 March 2014 16:58 >>> To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya >>> Cc: opensaf-devel@lists.sourceforge.net >>> Subject: [PATCH 1 of 1] amfd: use template class db to replace >>> patricia tree db >>> V2 [#713] >>> >>>osaf/services/saf/amf/amfd/include/db_template.h | 63 >>> >>>1 files changed, 63 insertions(+), 0 deletions(-) >>> >>> >>> diff --git a/osaf/services/saf/amf/amfd/include/db_template.h >>> b/osaf/services/saf/amf/amfd/include/db_template.h >>> new file mode 100644 >>> --- /dev/null >>> +++ b/osaf/services/saf/amf/amfd/include/db_template.h >>> @@ -0,0 +1,63 @@ >>> +/* -*- OpenSAF -*- >>> + * >>> + * (C) Copyright 2014 The OpenSAF Foundation >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> +but >>> + * WITHOUT ANY WARRANTY; without even the implied warranty of >>> MERCHANTABILITY >>> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are >>> +licensed >>> + * under the GNU Lesser General Public License Version 2.1, February 1999. >>> + * The complete license can be accessed from the following location: >>> + * http://opensource.org/licenses/lgpl-license.php >>> + * See the Copying file included with the OpenSAF distribution for >>> +full >>> + * licensing terms. >>> + * >>> + * Author(s): Ericsson AB >>> + * >>> + */ >>> +#ifndef DB_TEMPLATE_H >>> +#defineDB_TEMPLATE_H >>> + >>> +#include >>> +#include >>> + >>> +template >>> +class AmfDb { >>> + public: >>> + void insert(T *obj); >>> + void erase(T *obj); >>> + T *find(const SaNameT *name); >>> + >>> + typedef std::map AmfDbMap; >>> + typedef typename AmfDbMap::const_iterator const_iterator; >>> + >>> + const_iterator begin() const {return db.begin();} >>> + const_iterator end() const {return db.end();} >>> + >>> + private: >>> + AmfDbMap db; >>> +}; >>> + >>> +template >>> +void AmfDb::insert(T *obj) { >>> + std::string name((const char*)obj->name.value
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
"external" API normally needs to validate its input and further calling "internal" function these validations are not needed. Asserts can be used to detect bugs and are normally compiled away in release builds. I can add osafassert in these functions, but shouldn't the input already have been validated in calling functions? /BR HansN -Original Message- From: Anders Widell Sent: den 4 april 2014 16:27 To: Nagendra Kumar; Hans Nordebäck; Hans Feldt; Praveen Malviya Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] Just some thoughts regarding NULL pointers: When I write a function, I by default assume that pointers passed to it as arguments are not NULL. Only if the API documentation explicitly states that a NULL pointer is a valid input for that argument, then the function needs to handle it. Otherwise it is fine to crash (or assert) when receiving a NULL pointer. An assert is typically not needed, since the program will crash when dereferencing a NULL pointer anyway. Sometimes it is useful to allow function arguments to be NULL pointers. One typical example is the C library functions free() and realloc(), where it can be very handy to be able to pass a NULL pointer in case no memory has been allocated (yet). The API documentation here clearly says what happens when you pass a NULL pointer. regards, Anders Widell 2014-04-04 15:04, Nagendra Kumar skrev: > Ack with comment the functions added are not NULL safe. It crashed if passed > NULL. Do we want to have some checks ? > > Thanks > -Nagu > >> -Original Message- >> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >> Sent: 26 March 2014 16:58 >> To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 1 of 1] amfd: use template class db to replace >> patricia tree db >> V2 [#713] >> >> osaf/services/saf/amf/amfd/include/db_template.h | 63 >> >> 1 files changed, 63 insertions(+), 0 deletions(-) >> >> >> diff --git a/osaf/services/saf/amf/amfd/include/db_template.h >> b/osaf/services/saf/amf/amfd/include/db_template.h >> new file mode 100644 >> --- /dev/null >> +++ b/osaf/services/saf/amf/amfd/include/db_template.h >> @@ -0,0 +1,63 @@ >> +/* -*- OpenSAF -*- >> + * >> + * (C) Copyright 2014 The OpenSAF Foundation >> + * >> + * This program is distributed in the hope that it will be useful, >> +but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> MERCHANTABILITY >> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are >> +licensed >> + * under the GNU Lesser General Public License Version 2.1, February 1999. >> + * The complete license can be accessed from the following location: >> + * http://opensource.org/licenses/lgpl-license.php >> + * See the Copying file included with the OpenSAF distribution for >> +full >> + * licensing terms. >> + * >> + * Author(s): Ericsson AB >> + * >> + */ >> +#ifndef DB_TEMPLATE_H >> +#define DB_TEMPLATE_H >> + >> +#include >> +#include >> + >> +template >> +class AmfDb { >> + public: >> + void insert(T *obj); >> + void erase(T *obj); >> + T *find(const SaNameT *name); >> + >> + typedef std::map AmfDbMap; >> + typedef typename AmfDbMap::const_iterator const_iterator; >> + >> + const_iterator begin() const {return db.begin();} >> + const_iterator end() const {return db.end();} >> + >> + private: >> + AmfDbMap db; >> +}; >> + >> +template >> +void AmfDb::insert(T *obj) { >> + std::string name((const char*)obj->name.value, obj->name.length); >> + db[name] = obj; >> +} >> + >> +template >> +void AmfDb::erase(T *obj) { >> + std::string name((const char*)obj->name.value, obj->name.length); >> + typename AmfDbMap::iterator it = db.find(name); >> + db.erase(it); >> +} >> + >> +template >> +T *AmfDb::find(const SaNameT *dn) { >> + std::string name((const char*)dn->value, dn->length); >> + typename AmfDbMap::iterator it = db.find(name); >> + if (it == db.end()) >> +return NULL; >> + else >> +return it->second; >> +} >> + >> +#endif /* DB_TEMPLATE_H */ > -- > ___ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Just some thoughts regarding NULL pointers: When I write a function, I by default assume that pointers passed to it as arguments are not NULL. Only if the API documentation explicitly states that a NULL pointer is a valid input for that argument, then the function needs to handle it. Otherwise it is fine to crash (or assert) when receiving a NULL pointer. An assert is typically not needed, since the program will crash when dereferencing a NULL pointer anyway. Sometimes it is useful to allow function arguments to be NULL pointers. One typical example is the C library functions free() and realloc(), where it can be very handy to be able to pass a NULL pointer in case no memory has been allocated (yet). The API documentation here clearly says what happens when you pass a NULL pointer. regards, Anders Widell 2014-04-04 15:04, Nagendra Kumar skrev: > Ack with comment the functions added are not NULL safe. It crashed if passed > NULL. Do we want to have some checks ? > > Thanks > -Nagu > >> -Original Message- >> From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] >> Sent: 26 March 2014 16:58 >> To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya >> Cc: opensaf-devel@lists.sourceforge.net >> Subject: [PATCH 1 of 1] amfd: use template class db to replace patricia tree >> db >> V2 [#713] >> >> osaf/services/saf/amf/amfd/include/db_template.h | 63 >> >> 1 files changed, 63 insertions(+), 0 deletions(-) >> >> >> diff --git a/osaf/services/saf/amf/amfd/include/db_template.h >> b/osaf/services/saf/amf/amfd/include/db_template.h >> new file mode 100644 >> --- /dev/null >> +++ b/osaf/services/saf/amf/amfd/include/db_template.h >> @@ -0,0 +1,63 @@ >> +/* -*- OpenSAF -*- >> + * >> + * (C) Copyright 2014 The OpenSAF Foundation >> + * >> + * This program is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> MERCHANTABILITY >> + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed >> + * under the GNU Lesser General Public License Version 2.1, February 1999. >> + * The complete license can be accessed from the following location: >> + * http://opensource.org/licenses/lgpl-license.php >> + * See the Copying file included with the OpenSAF distribution for full >> + * licensing terms. >> + * >> + * Author(s): Ericsson AB >> + * >> + */ >> +#ifndef DB_TEMPLATE_H >> +#define DB_TEMPLATE_H >> + >> +#include >> +#include >> + >> +template >> +class AmfDb { >> + public: >> + void insert(T *obj); >> + void erase(T *obj); >> + T *find(const SaNameT *name); >> + >> + typedef std::map AmfDbMap; >> + typedef typename AmfDbMap::const_iterator const_iterator; >> + >> + const_iterator begin() const {return db.begin();} >> + const_iterator end() const {return db.end();} >> + >> + private: >> + AmfDbMap db; >> +}; >> + >> +template >> +void AmfDb::insert(T *obj) { >> + std::string name((const char*)obj->name.value, obj->name.length); >> + db[name] = obj; >> +} >> + >> +template >> +void AmfDb::erase(T *obj) { >> + std::string name((const char*)obj->name.value, obj->name.length); >> + typename AmfDbMap::iterator it = db.find(name); >> + db.erase(it); >> +} >> + >> +template >> +T *AmfDb::find(const SaNameT *dn) { >> + std::string name((const char*)dn->value, dn->length); >> + typename AmfDbMap::iterator it = db.find(name); >> + if (it == db.end()) >> +return NULL; >> + else >> +return it->second; >> +} >> + >> +#endif /* DB_TEMPLATE_H */ > -- > ___ > Opensaf-devel mailing list > Opensaf-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/opensaf-devel > > -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Hi, yes db.erase(name) is better. Duplicates in a map is not allowed so an item with the same key replaces the old item, so we should add a check for this. Adding const to insert and erase can be done but it will require const changes in every functions calling it. /Thanks HansN -Original Message- From: Zoran Milinkovic Sent: den 4 april 2014 15:48 To: praveen malviya; Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net Subject: RE: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] + typename AmfDbMap::iterator it = db.find(name); db.erase(it); The code above can be replaced with: db.erase(name); So, no extra checks for iterators. Regards, Zoran -Original Message- From: praveen malviya [mailto:praveen.malv...@oracle.com] Sent: den 4 april 2014 15:24 To: Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] Please find comments inline with [Praveen]. Thanks, Praveen On 26-Mar-14 4:57 PM, Hans Nordeback wrote: > osaf/services/saf/amf/amfd/include/db_template.h | 63 > > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, > +but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are > +licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for > +full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include > +#include > + > +template > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template > +void AmfDb::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + [Praveen] Patricia tree returns failure for duplicate entry or if addition itself fails. Don't we require such return code here? We check return code after addition. Also can we make argument as constant. > +template > +void AmfDb::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} [Praveen] If db.find returns NULL then this function should return error otherwise it crashes. something like this: if (it == db.end()) return ERROR; db.erase(it); Also can we make argument as constant. > + > +template > +T *AmfDb::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > +return NULL; > + else > +return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
+ typename AmfDbMap::iterator it = db.find(name); + db.erase(it); The code above can be replaced with: db.erase(name); So, no extra checks for iterators. Regards, Zoran -Original Message- From: praveen malviya [mailto:praveen.malv...@oracle.com] Sent: den 4 april 2014 15:24 To: Hans Nordebäck Cc: opensaf-devel@lists.sourceforge.net Subject: Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713] Please find comments inline with [Praveen]. Thanks, Praveen On 26-Mar-14 4:57 PM, Hans Nordeback wrote: > osaf/services/saf/amf/amfd/include/db_template.h | 63 > > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, > +but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are > +licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for > +full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include > +#include > + > +template > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template > +void AmfDb::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + [Praveen] Patricia tree returns failure for duplicate entry or if addition itself fails. Don't we require such return code here? We check return code after addition. Also can we make argument as constant. > +template > +void AmfDb::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} [Praveen] If db.find returns NULL then this function should return error otherwise it crashes. something like this: if (it == db.end()) return ERROR; db.erase(it); Also can we make argument as constant. > + > +template > +T *AmfDb::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > +return NULL; > + else > +return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
A null pointer is just a special case of a bad pointer which cannot be validated. An assert is ok to show the contract for calling this method Skickat från min Sony Xperia™-smartphone Nagendra Kumar skrev Ack with comment the functions added are not NULL safe. It crashed if passed NULL. Do we want to have some checks ? Thanks -Nagu > -Original Message- > From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] > Sent: 26 March 2014 16:58 > To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] amfd: use template class db to replace patricia tree > db > V2 [#713] > > osaf/services/saf/amf/amfd/include/db_template.h | 63 > > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include > +#include > + > +template > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template > +void AmfDb::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + > +template > +void AmfDb::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} > + > +template > +T *AmfDb::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > +return NULL; > + else > +return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Please find comments inline with [Praveen]. Thanks, Praveen On 26-Mar-14 4:57 PM, Hans Nordeback wrote: > osaf/services/saf/amf/amfd/include/db_template.h | 63 > > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include > +#include > + > +template > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template > +void AmfDb::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + [Praveen] Patricia tree returns failure for duplicate entry or if addition itself fails. Don't we require such return code here? We check return code after addition. Also can we make argument as constant. > +template > +void AmfDb::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} [Praveen] If db.find returns NULL then this function should return error otherwise it crashes. something like this: if (it == db.end()) return ERROR; db.erase(it); Also can we make argument as constant. > + > +template > +T *AmfDb::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > +return NULL; > + else > +return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Ack with comment the functions added are not NULL safe. It crashed if passed NULL. Do we want to have some checks ? Thanks -Nagu > -Original Message- > From: Hans Nordeback [mailto:hans.nordeb...@ericsson.com] > Sent: 26 March 2014 16:58 > To: hans.fe...@ericsson.com; Nagendra Kumar; Praveen Malviya > Cc: opensaf-devel@lists.sourceforge.net > Subject: [PATCH 1 of 1] amfd: use template class db to replace patricia tree > db > V2 [#713] > > osaf/services/saf/amf/amfd/include/db_template.h | 63 > > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include > +#include > + > +template > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template > +void AmfDb::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + > +template > +void AmfDb::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} > + > +template > +T *AmfDb::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > +return NULL; > + else > +return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1 of 1] amfd: use template class db to replace patricia tree db V2 [#713]
Ack I guess later we need to make this file common between director and node director. Thanks, Hans On 03/26/2014 12:27 PM, Hans Nordeback wrote: > osaf/services/saf/amf/amfd/include/db_template.h | 63 > > 1 files changed, 63 insertions(+), 0 deletions(-) > > > diff --git a/osaf/services/saf/amf/amfd/include/db_template.h > b/osaf/services/saf/amf/amfd/include/db_template.h > new file mode 100644 > --- /dev/null > +++ b/osaf/services/saf/amf/amfd/include/db_template.h > @@ -0,0 +1,63 @@ > +/* -*- OpenSAF -*- > + * > + * (C) Copyright 2014 The OpenSAF Foundation > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY > + * or FITNESS FOR A PARTICULAR PURPOSE. This file and program are licensed > + * under the GNU Lesser General Public License Version 2.1, February 1999. > + * The complete license can be accessed from the following location: > + * http://opensource.org/licenses/lgpl-license.php > + * See the Copying file included with the OpenSAF distribution for full > + * licensing terms. > + * > + * Author(s): Ericsson AB > + * > + */ > +#ifndef DB_TEMPLATE_H > +#define DB_TEMPLATE_H > + > +#include > +#include > + > +template > +class AmfDb { > + public: > + void insert(T *obj); > + void erase(T *obj); > + T *find(const SaNameT *name); > + > + typedef std::map AmfDbMap; > + typedef typename AmfDbMap::const_iterator const_iterator; > + > + const_iterator begin() const {return db.begin();} > + const_iterator end() const {return db.end();} > + > + private: > + AmfDbMap db; > +}; > + > +template > +void AmfDb::insert(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + db[name] = obj; > +} > + > +template > +void AmfDb::erase(T *obj) { > + std::string name((const char*)obj->name.value, obj->name.length); > + typename AmfDbMap::iterator it = db.find(name); > + db.erase(it); > +} > + > +template > +T *AmfDb::find(const SaNameT *dn) { > + std::string name((const char*)dn->value, dn->length); > + typename AmfDbMap::iterator it = db.find(name); > + if (it == db.end()) > +return NULL; > + else > +return it->second; > +} > + > +#endif /* DB_TEMPLATE_H */ > > -- ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel