git:master:cbd9fc13: htable: Add return code on successful deletion of item, update RPC commands with replies
7 Dec
2021
7 Dec
'21
7:53 a.m.
Module: kamailio
Branch: master
Commit: cbd9fc13ab11270df4b541afd9dc9b51517cdd12
URL: https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b5…
Author: Olle E. Johansson <oej(a)edvina.net>
Committer: Olle E. Johansson <oej(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Patch:
https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b5…
---
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;
}
7 Dec
7 Dec
8:13 a.m.
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/cbd9fc13ab11270df4b541afd9dc9b5…
Author: Olle E. Johansson <oej(a)edvina.net>
Committer: Olle E. Johansson <oej(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Patch:
https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b5…
---
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(a)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/
8:21 a.m.
On 7 Dec 2021, at 09:13, Daniel-Constantin Mierla
<miconda(a)gmail.com> wrote:
Hello,
this change has side effects and seems to break existing behaviour, one
that I spotted:
Sorry,
I looked around and did not find anything that depended on the return code.
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.
Will check
that case, must have missed it.
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.
Early morning
code. :-)
Thank you for checking.
/O
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/cbd9fc13ab11270df4b541afd9dc9b5…
Author: Olle E. Johansson <oej(a)edvina.net>
Committer: Olle E. Johansson <oej(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Patch:
https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b5…
---
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(a)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/
8:36 a.m.
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(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Author: Olle E. Johansson <oej(a)edvina.net>
Committer: Olle E. Johansson <oej(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Patch:
https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b5…
---
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(a)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/
8:48 a.m.
Hello,
On 07.12.21 09:36, Olle E. Johansson wrote:
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...
improvements are always welcome :-) ! The module was started in 2008,
probably the focus during that year was not much on documenting internal
c code ...
/**
*
*/
int ht_api_del_cell(str *hname, str *name)
I will reset it to return zero instead of one.
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:
ret = ht_api_rm_cell(...);
return (ret<0)?ret:0;
So the old function (by name) preserves the behaviour and you can use
the new function where you need it.
Cheers,
Daniel
/O
--
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/
On 7 Dec 2021, at 09:13, Daniel-Constantin Mierla
<miconda(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Author: Olle E. Johansson <oej(a)edvina.net>
Committer: Olle E. Johansson <oej(a)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/cbd9fc13ab11270df4b541afd9dc9b5…
Patch:
https://github.com/kamailio/kamailio/commit/cbd9fc13ab11270df4b541afd9dc9b5…
---
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(a)lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla -- www.asipto.com <http://www.asipto.com>
www.twitter.com/miconda <http://www.twitter.com/miconda> --
www.linkedin.com/in/miconda <http://www.linkedin.com/in/miconda>
Kamailio Advanced Training - Online
Feb 21-24, 2022 (America Timezone)
* https://www.asipto.com/sw/kamailio-advanced-training-online/
890
days inactive
890
days old
4 comments
2 participants
participants (2)
-
Daniel-Constantin Mierla -
Olle E. Johansson