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

[Olle E. Johansson]

Hi!

Return values seems poorly documented here, so it’s hard to fix stuff. ht_api_del_cell in api.c has literally empty docs and the API is not covered in the README...

/**
 *
 */
int ht_api_del_cell(str *hname, str *name)

I will reset it to return zero instead of one.

/O

> On 7 Dec 2021, at 09:13, Daniel-Constantin Mierla <miconda at gmail.com> wrote:
> 
> 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/
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20211207/f278a96c/attachment.htm>