Review Request 351: Indication manager [1/1] Indication manager

Jan Safranek reviewboard at gallagherhome.com
Fri May 17 07:12:38 UTC 2013



> On May 15, 2013, 6:36 p.m., Jan Safranek wrote:
> > src/indmanager/ind_manager.c, lines 621-622
> > <http://reviewboard-openlmi.rhcloud.com/r/351/diff/1/?file=2242#file2242line621>
> >
> >     I am afraid most of CMGet* function calls in this thread will leak memory as they do not run in context of any CIM operation (which would free it upon completion). Double check with valgrind!
> 
> Roman Rakus wrote:
>     I thought that CBAttachThread() will take care of it. But will check it.

CBAttachThread ensures that CMGet* won't fail. It won't de-allocate the copies returned from it. At least that's what I found in Pegasus implementation of CMPI.


> On May 15, 2013, 6:36 p.m., Jan Safranek wrote:
> > src/indmanager/ind_manager.c, line 502
> > <http://reviewboard-openlmi.rhcloud.com/r/351/diff/1/?file=2242#file2242line502>
> >
> >     CMSetProperty has CMPIValue* as the third argument, not CMPIInstance **.
> >     
> >     This applies to all CMSetProperty calls below.
> 
> Roman Rakus wrote:
>     Don't ask me why, but it works this way.

'Working' does not mean 'correct'. It works just accidentally because CMPIValue is union, having CMPIInstance* as member.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/351/#review411
-----------------------------------------------------------


On May 16, 2013, 1:59 p.m., Roman Rakus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-openlmi.rhcloud.com/r/351/
> -----------------------------------------------------------
> 
> (Updated May 16, 2013, 1:59 p.m.)
> 
> 
> Review request for OpenLMI Developers.
> 
> 
> Repository: openlmi-providers
> 
> 
> Description
> -------
> 
> Indication manager
> 
> Signed-off-by: Roman Rakus <rrakus at redhat.com>
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f25850e1640f52aed9f18c3bd343d41f7f5f21a4 
>   src/CMakeLists.txt 9f714d080d8541c368dedc235996e7a790930fff 
>   src/indmanager/CMakeLists.txt PRE-CREATION 
>   src/indmanager/ind_manager.h PRE-CREATION 
>   src/indmanager/ind_manager.c PRE-CREATION 
>   src/indmanager/openlmiindmanager.pc.in PRE-CREATION 
> 
> Diff: http://reviewboard-openlmi.rhcloud.com/r/351/diff/
> 
> 
> Testing
> -------
> 
> For testing purposes I used the following provider:
> #include <konkret/konkret.h>
> #include "LMI_AccountInstCreationIndication.h"
> 
> #include "ind_manager.h"
> #include <stdbool.h>
> 
> static const CMPIBroker* _cb = NULL;
> 
> static IMManager *im = NULL;
> IMError im_err = IM_ERR_OK;
> 
> static bool filter_checker(const CMPISelectExp *filter)
> {
>     //TODO - true checking
>     return true;
> }
> 
> // TODO - remove !
> #include <unistd.h>
> static bool spawner(void *data)
> {
>     //TODO - true checking
>     return true;
> }
> 
> // TODO - remove !
> #include <unistd.h>
> static bool spawner(void *data)
> {
>     //TODO - inotify
>     sleep(10);
>     return true;
> }
> 
> static void LMI_AccountInstCreationIndicationInitialize()
> {
>     // startup indication manager
>     im = im_create_manager(NULL, filter_checker, true, spawner,
>                            IM_IND_CREATION, (CMPIBroker *)_cb, &im_err);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationIndicationCleanup(
>     CMPIIndicationMI* mi,
>     const CMPIContext* cc,
>     CMPIBoolean term)
> {
>     // destroy indication manager
>     im_destroy_manager(im, cc, &im_err);
>     CMReturn(CMPI_RC_OK);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationAuthorizeFilter(
>     CMPIIndicationMI* mi,
>     const CMPIContext* cc,
>     const CMPISelectExp* se,
>     const char* ns,
>     const CMPIObjectPath* op,
>     const char* user)
> {
>     // verify filter
>     im_verify_filter(im, se, cc, &im_err);
>     CMReturn(CMPI_RC_OK);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationMustPoll(
>     CMPIIndicationMI* mi,
>     const CMPIContext* cc,
>     const CMPISelectExp* se,
>     const char* ns, 
>     const CMPIObjectPath* op)
> {
>   // yes, this is not supported :(
>     CMReturn(CMPI_RC_ERR_NOT_SUPPORTED);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationActivateFilter(
>     CMPIIndicationMI* mi,
>     const CMPIContext* cc,
>     const CMPISelectExp* se,
>     const char* ns,
>     const CMPIObjectPath* op,
>     CMPIBoolean firstActivation)
> {
>     // verify filter and add it via indication manager
>     im_verify_filter(im, se, cc, &im_err);
>     im_add_filter(im, (CMPISelectExp*)se, cc, &im_err);
>     CMReturn(CMPI_RC_OK);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationDeActivateFilter(
>     CMPIIndicationMI* mi,
>     const CMPIContext* cc,
>     const CMPISelectExp* se,
>     const char* ns,
>     const CMPIObjectPath* op,
>     CMPIBoolean lastActivation)
> {
>     // remove filter from indication manager
>     im_remove_filter(im, se, cc, &im_err);
>     CMReturn(CMPI_RC_OK);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationEnableIndications(
>     CMPIIndicationMI* mi, 
>     const CMPIContext* cc)
> {
>     // start indications
>     im_start_ind(im, cc, &im_err);
>     CMReturn(CMPI_RC_OK);
> }
> 
> static CMPIStatus LMI_AccountInstCreationIndicationDisableIndications(
>     CMPIIndicationMI* mi, 
>     const CMPIContext* cc)
> {
>     // stop indications
>     im_stop_ind(im, cc, &im_err);
>     CMReturn(CMPI_RC_OK);
> }
> 
> CMIndicationMIStub(
>     LMI_AccountInstCreationIndication,
>     LMI_AccountInstCreationIndication, 
>     _cb, 
>     LMI_AccountInstCreationIndicationInitialize())
> 
> KONKRET_REGISTRATION(
>     "root/cimv2",
>     "LMI_AccountInstCreationIndication",
>     "LMI_AccountInstCreationIndication",
>     "indication")
> 
> 
> Thanks,
> 
> Roman Rakus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/openlmi-devel/attachments/20130517/7de10e1a/attachment.html>


More information about the openlmi-devel mailing list