[sr-dev] git:master:cbd9fc13: htable: Add return code on successful deletion of item, update RPC commands with replies

[Daniel-Constantin Mierla]

Hello,

this change has side effects and seems to break existing behaviour, one
that I spotted:

src/modules/htable/ht_dmq.c
327:            if (ht_dmq_replay_action(action, &htname, &cname, type,
&val, mode)!=0) {
                        LM_ERR("failed to replay action\n");
                        goto error;

Inside ht_dmq_replay_action() it is:

         return ht_del_cell(ht, cname);

Because ht_del_cell() was changed to return 1 in case of successful
deletion, practically ends up in error case inside the ht_dmq.c code.

There are other places where the del function is used, I haven't checked
all of them, but the current form does not seem correct as it is.

Cheers,
Daniel

On 07.12.21 08:53, Olle E. Johansson wrote:
> Module: kamailio
> Branch: master
> Commit: cbd9fc13ab11270df4b541afd9dc9b51517cdd12
> URL: https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12
>
> Author: Olle E. Johansson <oej at edvina.net>
> Committer: Olle E. Johansson <oej at edvina.net>
> Date: 2021-12-07T08:52:08+01:00
>
> htable: Add return code on successful deletion of item, update RPC commands with replies
>
> ---
>
> Modified: src/modules/htable/ht_api.c
> Modified: src/modules/htable/htable.c
>
> ---
>
> Diff:  https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12.diff
> Patch: https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12.patch
>
> ---
>
> diff --git a/src/modules/htable/ht_api.c b/src/modules/htable/ht_api.c
> index d4310afa3d..45623e0165 100644
> --- a/src/modules/htable/ht_api.c
> +++ b/src/modules/htable/ht_api.c
> @@ -655,6 +655,13 @@ static void ht_cell_unlink(ht_t *ht, int idx, ht_cell_t *it)
>  	ht->entries[idx].esize--;
>  }
>  
> +/* Delete htable entry
> +
> +Return:
> +	-1 on error in argument
> +	0 on entry not found
> +	1 on entry found and deleted
> +*/
>  int ht_del_cell(ht_t *ht, str *name)
>  {
>  	unsigned int idx;
> @@ -689,7 +696,7 @@ int ht_del_cell(ht_t *ht, str *name)
>  			ht_cell_unlink(ht, idx, it);
>  			ht_slot_unlock(ht, idx);
>  			ht_cell_free(it);
> -			return 0;
> +			return 1;
>  		}
>  		it = it->next;
>  	}
> diff --git a/src/modules/htable/htable.c b/src/modules/htable/htable.c
> index a20b3d6e8d..d491e2e767 100644
> --- a/src/modules/htable/htable.c
> +++ b/src/modules/htable/htable.c
> @@ -1438,6 +1438,7 @@ static const char* htable_store_doc[2] = {
>  static void htable_rpc_delete(rpc_t* rpc, void* c) {
>  	str htname, keyname;
>  	ht_t *ht;
> +	int res;
>  
>  	if (rpc->scan(c, "SS", &htname, &keyname) < 2) {
>  		rpc->fault(c, 500, "Not enough parameters (htable name & key name");
> @@ -1453,7 +1454,17 @@ static void htable_rpc_delete(rpc_t* rpc, void* c) {
>  		LM_ERR("dmq replication failed\n");
>  	}
>  
> -	ht_del_cell(ht, &keyname);
> +	res = ht_del_cell(ht, &keyname);
> +
> +	if (res  == -1) {
> +		rpc->fault(c, 500, "Internal error");
> +		return;
> +	} else if (res == 0) {
> +		rpc->fault(c, 404, "Key not found in htable.");
> +		return;
> +	}
> +	rpc->rpl_printf(c, "Ok. Key deleted.");
> +	return;
>  }
>  
>  /*! \brief RPC htable.get command to get one item */
> @@ -1561,7 +1572,7 @@ static void htable_rpc_sets(rpc_t* rpc, void* c) {
>  		rpc->fault(c, 500, "Failed to set the item");
>  		return;
>  	}
> -
> +	rpc->rpl_printf(c, "Ok. Key set to new value.");
>  	return;
>  }
>  
> @@ -1596,7 +1607,7 @@ static void htable_rpc_seti(rpc_t* rpc, void* c) {
>  		rpc->fault(c, 500, "Failed to set the item");
>  		return;
>  	}
> -
> +	rpc->rpl_printf(c, "Ok. Key set to new value.");
>  	return;
>  }
>  
>
>
> _______________________________________________
> Kamailio (SER) - Development Mailing List
> sr-dev at lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio Advanced Training - Online
  Feb 21-24, 2022 (America Timezone)
  * https://www.asipto.com/sw/kamailio-advanced-training-online/