[Libvirt-cim] [PATCH 1 of 3] [CU] Turn std_indication's awesome knob to eleven

[Jay Gagnon]

Dan Smith wrote:
> JG>  static CMPIStatus trigger(struct std_indication_ctx *ctx,
> JG>                            const CMPIContext *context)
> JG>  {
> JG> -        if (ctx->handler->trigger_fn == NULL)
> JG> +        if (ctx->handler == NULL || ctx->handler->trigger_fn == NULL)
> JG>                  return (CMPIStatus){CMPI_RC_OK, NULL};
>
> Maybe I'm misreading the diff, but do you check for enabled/activated
> on trigger?
>   
I don't here, but when the trigger loop tries to use stdi_deliver to 
send off its indication, I check there.  I guess it would be easy to 
check here, and save a bit of work, so I can add that in.
> JG> +CMPIStatus stdi_activate_filter(CMPIIndicationMI* mi,
> JG> +                                const CMPIContext* ctx,
> JG> +                                const CMPISelectExp* se,
> JG> +                                const char *ns,
> JG> +                                const CMPIObjectPath* op,
> JG> +                                CMPIBoolean first)
> JG> +{
> JG> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> JG> +        struct std_indication_ctx *_ctx;
> JG> +        _ctx = (struct std_indication_ctx *)mi->hdl;
> JG> +        char *cn = NULL;
> JG> +        
> JG> +        _ctx = (struct std_indication_ctx *)mi->hdl;
>
> Looks like you have the _ctx initialization twice in here.  Remove the
> first to avoid intermixed statements and declarations.
>   
Ah, the dreaded oh-look-I-copied-and-pasted-and-didn't-even-proofread-it!
> JG> +CMPIStatus stdi_deactivate_filter(CMPIIndicationMI* mi,
> JG> +                                  const CMPIContext* ctx,
> JG> +                                  const CMPISelectExp* se,
> JG> +                                  const  char *ns,
> JG> +                                  const CMPIObjectPath* op,
> JG> +                                  CMPIBoolean last)
> JG> +{
> JG> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> JG> +        struct std_indication_ctx *_ctx;
> JG> +        _ctx = (struct std_indication_ctx *)mi->hdl;
> JG> +        char *cn = NULL;
> JG> +
> JG> +        _ctx = (struct std_indication_ctx *)mi->hdl;
>
> Here too.
>
> JG> +_EI_RTYPE stdi_enable_indications (CMPIIndicationMI* mi,
> JG> +                                   const CMPIContext *ctx)
> JG> +{
> JG> +        struct std_indication_ctx *_ctx;
> JG> +        _ctx = (struct std_indication_ctx *)mi->hdl;
> JG> +
> JG> +        CU_DEBUG("enabling indications");
>
> I think this debug would be a lot more useful if it had the indication
> type in it.  Would that be much trouble?
>   
I *think* it should be easy enough.  With the mi and the context I 
should be able to find it somewhere.
> JG> +_EI_RTYPE stdi_disable_indications (CMPIIndicationMI* mi,
> JG> +                                    const CMPIContext *ctx)
> JG> +{
> JG> +        struct std_indication_ctx *_ctx;
> JG> +        _ctx = (struct std_indication_ctx *)mi->hdl;
> JG> +
> JG> +        CU_DEBUG("disabling indications");
>
> This one too.
>
> Otherwise this looks fantastic...Thanks!
>
>   
>   


-- 

-Jay