From notifications@github.com Thu Jan 21 19:05:23 2021 From: Nicolas C To: sr-dev@lists.kamailio.org Subject: [sr-dev] [kamailio/kamailio] [kamailio 5.4.3] xavp_rm_by_index removes (N+1) first indexes instead of index N (#2604) Date: Thu, 21 Jan 2021 10:05:16 -0800 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1827946122==" --===============1827946122== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable As previously discussed on the mailing list, this is a bug. I have a fix that works, so I'll provide a pull request. ### Description I=E2=80=99m trying to use function xavp_rm_by_index to remove a specific inde= x from a given xavp. I=E2=80=99m observing an unexpected behaviour, I think it might be a bug. Instead of just removing the index, the function removes the index and all ot= hers before it. E.g : if called with =C2=AB idx =3D 1 =C2=BB, it removes indexes 0 and 1. Reading the code of xavp_rm_internal (which is called by xavp_rm_by_index), I= don=E2=80=99t think it behaves at it should : ``` /* Remove xavps * idx: <0 remove all xavps with the same name * >=3D0 remove only the specified index xavp * Returns number of xavps that were deleted */ ``` For example when I do this : ``` LM_ERR("COUNT BEFORE: %d\n", xavp_count(rname, NULL)); int rm_count =3D xavp_rm_by_index(rname, 1, NULL); // TEST LM_ERR("REMOVED: %d\n", rm_count); LM_ERR("COUNT AFTER: %d\n", xavp_count(rname, NULL)); ``` =C2=AB rm_count =C2=BB returned is 1 (which is expected). However, =C2=AB cou= nt after =C2=BB is decremented by 2. And when I look at the contents of my xavp, I notice that the first two eleme= nts have been removed. Likewise if xavp_rm_by_index is invoked with idx =3D 2 =3D> the first 3 eleme= nts are removed. ### Troubleshooting #### Reproduction Here is some code that shows the issue : 1) a test cmd_function in a module defined as follows : ``` static int was_test(struct sip_msg *msg, char *param1, char *param2); static cmd_export_t cmds[] =3D { {"was_test", (cmd_function)was_test, 2, 0, 0, ANY_ROUTE}, {0, 0, 0, 0, 0, 0} }; /** * For testing purposes. */ static int was_test(struct sip_msg *msg, char *param1, char *param2) { LM_WARN("Called with param1: [%s], param2: [%s]\n", param1, param2); str my_str; my_str.s =3D param1; my_str.len =3D strlen(param1); int idx =3D atoi(param2); str *rname =3D &my_str; LM_ERR("COUNT BEFORE: %d\n", xavp_count(rname, NULL)); int rm_count =3D xavp_rm_by_index(rname, idx, NULL); LM_ERR("REMOVED: %d\n", rm_count); LM_ERR("COUNT AFTER: %d\n", xavp_count(rname, NULL)); return 1; } ``` 2) Kamailio script : ``` request_route { # push 4 rows to xavp "test": $xavp(test=3D>a) =3D "3.a"; $xavp(test[0]=3D>b) =3D "3.b"; $xavp(test=3D>a) =3D "2.a"; $xavp(test[0]=3D>b) =3D "2.b"; $xavp(test=3D>a) =3D "1.a"; $xavp(test[0]=3D>b) =3D "1.b"; $xavp(test=3D>a) =3D "0.a"; $xavp(test[0]=3D>b) =3D "0.b"; # then try to call xavp_rm_by_index with a non-zero idx xlog("L_INFO", "BEFORE\n"); xlog("L_INFO", "test[*]: [$(was_xavp_serialize(test)[*])]\n"); pv_xavp_print(); was_test("test", "1"); // expecting only row 1 to be removed (1.a / 1.= b) xlog("L_INFO", "AFTER\n"); xlog("L_INFO", "test[*]: [$(was_xavp_serialize(test)[*])]\n"); pv_xavp_print(); } ``` #### Log Messages ``` 2(16546) INFO: {1 INVITE 1-22211(a)10.31.22.1}