<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hello,<br>
</p>
<div class="moz-cite-prefix">On 07.12.21 09:36, Olle E. Johansson
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:0057893F-F1A1-4C08-8D86-3105E543573B@edvina.net">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
Hi!
<div class=""><br class="">
</div>
<div class="">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...</div>
</blockquote>
<p>improvements are always welcome :-) ! The module was started in
2008, probably the focus during that year was not much on
documenting internal c code ...<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:0057893F-F1A1-4C08-8D86-3105E543573B@edvina.net">
<div class="">
<div class=""><br class="">
</div>
<div class="">
<div style="background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; font-size: 12px; line-height: 18px; white-space: pre;" class=""><div class=""><span style="color: #008000;" class="">/**</span></div><div class=""><span style="color: #008000;" class=""> *</span></div><div class=""><span style="color: #008000;" class=""> */</span></div><div class=""><span style="color: #0000ff;" class="">int</span> <span style="color: #795e26;" class="">ht_api_del_cell</span>(<span style="color: #267f99;" class="">str</span> *<span style="color: #001080;" class="">hname</span>, <span style="color: #267f99;" class="">str</span> *<span style="color: #001080;" class="">name</span>)</div><div class="">
</div><div class="">I will reset it to return zero instead of one.</div></div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Probably is better to rename the function ht_api_del_cell() to
something else like ht_api_rm_cell(), which returns 1 on removal,
then (re-)add ht_api_del_cell() as a wrapper around the new
function, with code like:</p>
<p>ret = ht_api_rm_cell(...);</p>
<p>return (ret<0)?ret:0;</p>
<p>So the old function (by name) preserves the behaviour and you can
use the new function where you need it.<br>
</p>
<p>Cheers,<br>
Daniel<br>
</p>
<blockquote type="cite"
cite="mid:0057893F-F1A1-4C08-8D86-3105E543573B@edvina.net">
<div class="">
<div class="">
<div style="background-color: rgb(255, 255, 255); font-family: Menlo, Monaco, "Courier New", monospace; font-size: 12px; line-height: 18px; white-space: pre;" class=""><div class="">
</div><div class="">/O</div></div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">On 7 Dec 2021, at 09:13, Daniel-Constantin
Mierla <<a href="mailto:miconda@gmail.com"
class="moz-txt-link-freetext" moz-do-not-send="true">miconda@gmail.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">Hello,<br class="">
<br class="">
this change has side effects and seems to break
existing behaviour, one<br class="">
that I spotted:<br class="">
<br class="">
src/modules/htable/ht_dmq.c<br class="">
327: if (ht_dmq_replay_action(action,
&htname, &cname, type,<br class="">
&val, mode)!=0) {<br class="">
LM_ERR("failed to replay
action\n");<br class="">
goto error;<br class="">
<br class="">
Inside ht_dmq_replay_action() it is:<br class="">
<br class="">
return ht_del_cell(ht, cname);<br class="">
<br class="">
Because ht_del_cell() was changed to return 1 in case
of successful<br class="">
deletion, practically ends up in error case inside the
ht_dmq.c code.<br class="">
<br class="">
There are other places where the del function is used,
I haven't checked<br class="">
all of them, but the current form does not seem
correct as it is.<br class="">
<br class="">
Cheers,<br class="">
Daniel<br class="">
<br class="">
On 07.12.21 08:53, Olle E. Johansson wrote:<br
class="">
<blockquote type="cite" class="">Module: kamailio<br
class="">
Branch: master<br class="">
Commit: cbd9fc13ab11270df4b541afd9dc9b51517cdd12<br
class="">
URL: <a
href="https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12"
class="moz-txt-link-freetext"
moz-do-not-send="true">https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12</a><br
class="">
<br class="">
Author: Olle E. Johansson <<a
href="mailto:oej@edvina.net"
class="moz-txt-link-freetext"
moz-do-not-send="true">oej@edvina.net</a>><br
class="">
Committer: Olle E. Johansson <<a
href="mailto:oej@edvina.net"
class="moz-txt-link-freetext"
moz-do-not-send="true">oej@edvina.net</a>><br
class="">
Date: 2021-12-07T08:52:08+01:00<br class="">
<br class="">
htable: Add return code on successful deletion of
item, update RPC commands with replies<br class="">
<br class="">
---<br class="">
<br class="">
Modified: src/modules/htable/ht_api.c<br class="">
Modified: src/modules/htable/htable.c<br class="">
<br class="">
---<br class="">
<br class="">
Diff: <a
href="https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12.diff"
class="moz-txt-link-freetext"
moz-do-not-send="true">https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12.diff</a><br
class="">
Patch: <a
href="https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12.patch"
class="moz-txt-link-freetext"
moz-do-not-send="true">https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b51517cdd12.patch</a><br
class="">
<br class="">
---<br class="">
<br class="">
diff --git a/src/modules/htable/ht_api.c
b/src/modules/htable/ht_api.c<br class="">
index d4310afa3d..45623e0165 100644<br class="">
--- a/src/modules/htable/ht_api.c<br class="">
+++ b/src/modules/htable/ht_api.c<br class="">
@@ -655,6 +655,13 @@ static void ht_cell_unlink(ht_t
*ht, int idx, ht_cell_t *it)<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>ht->entries[idx].esize--;<br
class="">
}<br class="">
<br class="">
+/* Delete htable entry<br class="">
+<br class="">
+Return:<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>-1
on error in argument<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>0
on entry not found<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>1
on entry found and deleted<br class="">
+*/<br class="">
int ht_del_cell(ht_t *ht, str *name)<br class="">
{<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>unsigned
int idx;<br class="">
@@ -689,7 +696,7 @@ int ht_del_cell(ht_t *ht, str
*name)<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ht_cell_unlink(ht,
idx, it);<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ht_slot_unlock(ht,
idx);<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>ht_cell_free(it);<br
class="">
-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return
0;<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return
1;<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>}<br
class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>it
= it->next;<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>}<br
class="">
diff --git a/src/modules/htable/htable.c
b/src/modules/htable/htable.c<br class="">
index a20b3d6e8d..d491e2e767 100644<br class="">
--- a/src/modules/htable/htable.c<br class="">
+++ b/src/modules/htable/htable.c<br class="">
@@ -1438,6 +1438,7 @@ static const char*
htable_store_doc[2] = {<br class="">
static void htable_rpc_delete(rpc_t* rpc, void* c) {<br
class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>str
htname, keyname;<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>ht_t
*ht;<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>int
res;<br class="">
<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>if
(rpc->scan(c, "SS", &htname, &keyname)
< 2) {<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>rpc->fault(c,
500, "Not enough parameters (htable name & key
name");<br class="">
@@ -1453,7 +1454,17 @@ static void
htable_rpc_delete(rpc_t* rpc, void* c) {<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>LM_ERR("dmq
replication failed\n");<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>}<br
class="">
<br class="">
-<span class="Apple-tab-span" style="white-space:pre"> </span>ht_del_cell(ht,
&keyname);<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>res
= ht_del_cell(ht, &keyname);<br class="">
+<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>if
(res == -1) {<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>rpc->fault(c,
500, "Internal error");<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>}
else if (res == 0) {<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>rpc->fault(c,
404, "Key not found in htable.");<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>}<br
class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>rpc->rpl_printf(c,
"Ok. Key deleted.");<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
}<br class="">
<br class="">
/*! \brief RPC htable.get command to get one item */<br
class="">
@@ -1561,7 +1572,7 @@ static void
htable_rpc_sets(rpc_t* rpc, void* c) {<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>rpc->fault(c,
500, "Failed to set the item");<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>}<br
class="">
-<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>rpc->rpl_printf(c,
"Ok. Key set to new value.");<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
}<br class="">
<br class="">
@@ -1596,7 +1607,7 @@ static void
htable_rpc_seti(rpc_t* rpc, void* c) {<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>rpc->fault(c,
500, "Failed to set the item");<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>}<br
class="">
-<br class="">
+<span class="Apple-tab-span" style="white-space:pre"> </span>rpc->rpl_printf(c,
"Ok. Key set to new value.");<br class="">
<span class="Apple-tab-span" style="white-space:pre"> </span>return;<br
class="">
}<br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br
class="">
Kamailio (SER) - Development Mailing List<br
class="">
<a href="mailto:sr-dev@lists.kamailio.org"
class="moz-txt-link-freetext"
moz-do-not-send="true">sr-dev@lists.kamailio.org</a><br
class="">
<a class="moz-txt-link-freetext" href="https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev">https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev</a><br class="">
</blockquote>
<br class="">
-- <br class="">
Daniel-Constantin Mierla -- <a
href="http://www.asipto.com" class=""
moz-do-not-send="true">www.asipto.com</a><br
class="">
<a href="http://www.twitter.com/miconda" class=""
moz-do-not-send="true">www.twitter.com/miconda</a>
-- <a href="http://www.linkedin.com/in/miconda"
class="" moz-do-not-send="true">www.linkedin.com/in/miconda</a><br
class="">
Kamailio Advanced Training - Online<br class="">
Feb 21-24, 2022 (America Timezone)<br class="">
* <a
href="https://www.asipto.com/sw/kamailio-advanced-training-online/"
class="moz-txt-link-freetext" moz-do-not-send="true">https://www.asipto.com/sw/kamailio-advanced-training-online/</a><br
class="">
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</blockquote>
<pre class="moz-signature" cols="72">--
Daniel-Constantin Mierla -- <a class="moz-txt-link-abbreviated" href="http://www.asipto.com">www.asipto.com</a>
<a class="moz-txt-link-abbreviated" href="http://www.twitter.com/miconda">www.twitter.com/miconda</a> -- <a class="moz-txt-link-abbreviated" href="http://www.linkedin.com/in/miconda">www.linkedin.com/in/miconda</a>
Kamailio Advanced Training - Online
Feb 21-24, 2022 (America Timezone)
* <a class="moz-txt-link-freetext" href="https://www.asipto.com/sw/kamailio-advanced-training-online/">https://www.asipto.com/sw/kamailio-advanced-training-online/</a></pre>
</body>
</html>