Skip to content

Commit

Permalink
Code quality: Use str_match() instead of !str_strcmp()
Browse files Browse the repository at this point in the history
Reasons behind this change:
    - better performance: str_strcmp() includes lots of redundant checks
      and performs worse than the simplistic str_match()

    - safer code: although by making this change we increase the chance
      of a segfault (if either "a" or "b" strings are NULL), the crash
      will immediately indicate the coding bug and will be easy to
      troubleshoot.  By the same argument, nobody complains that
      memset() or strcmp() include zero NULL checks: they don't have to!

    - more meaningful code: logically speaking, in most cases, the
      programmer simply intends to _match_ two strings, rather than to
      order them lexicographically...

(cherry picked from commit 765a84b)
  • Loading branch information
liviuchircu committed Nov 4, 2020
1 parent 8032702 commit d004a6c
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 64 deletions.
2 changes: 1 addition & 1 deletion cachedb/cachedb_dict.c
Expand Up @@ -259,7 +259,7 @@ cdb_pair_t *cdb_dict_fetch(const cdb_key_t *key, const cdb_dict_t *dict)
pair = list_entry(_, cdb_pair_t, list);

if ((key->is_pk && pair->key.is_pk)
|| !str_strcmp(&pair->key.name, &key->name))
|| str_match(&pair->key.name, &key->name))
return pair;
}

Expand Down
82 changes: 41 additions & 41 deletions lib/test/test_csv.c
Expand Up @@ -30,43 +30,43 @@ void test_csv_simple(void)
csv_record *ret;

ret = parse_csv_record(_str(""));
ok(!str_strcmp(&ret->s, _str("")));
ok(str_match(&ret->s, _str("")));
ok(!ret->next);
free_csv_record(ret);

ret = parse_csv_record(_str("\t\r\n "));
ok(!str_strcmp(&ret->s, _str("")));
ok(str_match(&ret->s, _str("")));
ok(!ret->next);
free_csv_record(ret);

ret = parse_csv_record(_str(" a b "));
ok(!str_strcmp(&ret->s, _str("a b")));
ok(str_match(&ret->s, _str("a b")));
ok(!ret->next);
free_csv_record(ret);

ret = parse_csv_record(_str(" a , "));
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str("")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str("")));
ok(!ret->next->next);
free_csv_record(ret);

ret = parse_csv_record(_str(" , a "));
ok(!str_strcmp(&ret->s, _str("")));
ok(!str_strcmp(&ret->next->s, _str("a")));
ok(str_match(&ret->s, _str("")));
ok(str_match(&ret->next->s, _str("a")));
ok(!ret->next->next);
free_csv_record(ret);

ret = parse_csv_record(_str("a,b,c"));
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str("b")));
ok(!str_strcmp(&ret->next->next->s, _str("c")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str("b")));
ok(str_match(&ret->next->next->s, _str("c")));
ok(!ret->next->next->next);
free_csv_record(ret);

ret = parse_csv_record(_str(" a, \"b\" , \" c "));
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str("\"b\"")));
ok(!str_strcmp(&ret->next->next->s, _str("\" c")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str("\"b\"")));
ok(str_match(&ret->next->next->s, _str("\" c")));
ok(!ret->next->next->next);
free_csv_record(ret);
}
Expand All @@ -84,50 +84,50 @@ void test_csv_rfc_4180(void)
ok(!_parse_csv_record(_str("a,b,c\t"), CSV_RFC_4180));

ret = _parse_csv_record(_str(""), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("")));
ok(str_match(&ret->s, _str("")));
ok(!ret->next);
free_csv_record(ret);

ret = _parse_csv_record(_str(" a "), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str(" a ")));
ok(str_match(&ret->s, _str(" a ")));
ok(!ret->next);
free_csv_record(ret);

ret = _parse_csv_record(_str(" \r\n"), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str(" ")));
ok(str_match(&ret->s, _str(" ")));
ok(!ret->next);
free_csv_record(ret);

ret = _parse_csv_record(_str(","), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("")));
ok(!str_strcmp(&ret->next->s, _str("")));
ok(str_match(&ret->s, _str("")));
ok(str_match(&ret->next->s, _str("")));
ok(!ret->next->next);
free_csv_record(ret);

ret = _parse_csv_record(_str("a,,,"), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str("")));
ok(!str_strcmp(&ret->next->next->s, _str("")));
ok(!str_strcmp(&ret->next->next->next->s, _str("")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str("")));
ok(str_match(&ret->next->next->s, _str("")));
ok(str_match(&ret->next->next->next->s, _str("")));
ok(!ret->next->next->next->next);
free_csv_record(ret);

ret = _parse_csv_record(_str(" , a "), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str(" ")));
ok(!str_strcmp(&ret->next->s, _str(" a ")));
ok(str_match(&ret->s, _str(" ")));
ok(str_match(&ret->next->s, _str(" a ")));
ok(!ret->next->next);
free_csv_record(ret);

ret = _parse_csv_record(_str(" a , \r\n"), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str(" a ")));
ok(!str_strcmp(&ret->next->s, _str(" ")));
ok(str_match(&ret->s, _str(" a ")));
ok(str_match(&ret->next->s, _str(" ")));
ok(!ret->next->next);
free_csv_record(ret);

ret = _parse_csv_record(_str("a,b,c"), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str("b")));
ok(!str_strcmp(&ret->next->next->s, _str("c")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str("b")));
ok(str_match(&ret->next->next->s, _str("c")));
ok(!ret->next->next->next);
free_csv_record(ret);

Expand All @@ -137,30 +137,30 @@ void test_csv_rfc_4180(void)
ok(!_parse_csv_record(_str("\"a\"a,b"), CSV_RFC_4180));

ret = _parse_csv_record(_str("\"a\""), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("a")));
ok(str_match(&ret->s, _str("a")));
ok(!ret->next);
free_csv_record(ret);

ret = _parse_csv_record(_str("realm=\"etc.example.com\","
"nonce=\"B5CFDSFDGD14992F\",opaque=\"opaq\","
"qop=\"auth\",algorithm=MD5"), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("realm=\"etc.example.com\"")));
ok(!str_strcmp(&ret->next->s, _str("nonce=\"B5CFDSFDGD14992F\"")));
ok(!str_strcmp(&ret->next->next->s, _str("opaque=\"opaq\"")));
ok(!str_strcmp(&ret->next->next->next->s, _str("qop=\"auth\"")));
ok(!str_strcmp(&ret->next->next->next->next->s, _str("algorithm=MD5")));
ok(str_match(&ret->s, _str("realm=\"etc.example.com\"")));
ok(str_match(&ret->next->s, _str("nonce=\"B5CFDSFDGD14992F\"")));
ok(str_match(&ret->next->next->s, _str("opaque=\"opaq\"")));
ok(str_match(&ret->next->next->next->s, _str("qop=\"auth\"")));
ok(str_match(&ret->next->next->next->next->s, _str("algorithm=MD5")));
ok(!ret->next->next->next->next->next);
free_csv_record(ret);

ret = _parse_csv_record(_str("a,\"\tb\""), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str("\tb")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str("\tb")));
ok(!ret->next->next);
free_csv_record(ret);

ret = _parse_csv_record(_str("\"a\", bc "), CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("a")));
ok(!str_strcmp(&ret->next->s, _str(" bc ")));
ok(str_match(&ret->s, _str("a")));
ok(str_match(&ret->next->s, _str(" bc ")));
ok(!ret->next->next);
free_csv_record(ret);

Expand All @@ -171,8 +171,8 @@ void test_csv_rfc_4180(void)

ret = _parse_csv_record(_str("\"\"\"a\"\" \r\"\"\t\nb\"\" \",\"c\"\r\n"),
CSV_RFC_4180);
ok(!str_strcmp(&ret->s, _str("\"a\" \r\"\t\nb\" ")));
ok(!str_strcmp(&ret->next->s, _str("c")));
ok(str_match(&ret->s, _str("\"a\" \r\"\t\nb\" ")));
ok(str_match(&ret->next->s, _str("c")));
ok(!ret->next->next);
free_csv_record(ret);
}
Expand Down
16 changes: 8 additions & 8 deletions modules/acc/acc_extra.c
Expand Up @@ -197,16 +197,16 @@ static struct acc_extra** extra_str2bkend(str* bkend)
str aaa_bkend_s = str_init("aaa");
str evi_bkend_s = str_init("evi");

if (!str_strcmp(bkend, &log_bkend_s))
if (str_match(bkend, &log_bkend_s))
return &log_extra_tags;

if (!str_strcmp(bkend, &db_bkend_s))
if (str_match(bkend, &db_bkend_s))
return &db_extra_tags;

if (!str_strcmp(bkend, &aaa_bkend_s))
if (str_match(bkend, &aaa_bkend_s))
return &aaa_extra_tags;

if (!str_strcmp(bkend, &evi_bkend_s))
if (str_match(bkend, &evi_bkend_s))
return &evi_extra_tags;

return NULL;
Expand All @@ -219,16 +219,16 @@ static struct acc_extra** leg_str2bkend(str* bkend)
str aaa_bkend_s = str_init("aaa");
str evi_bkend_s = str_init("evi");

if (!str_strcmp(bkend, &log_bkend_s))
if (str_match(bkend, &log_bkend_s))
return &log_leg_tags;

if (!str_strcmp(bkend, &db_bkend_s))
if (str_match(bkend, &db_bkend_s))
return &db_leg_tags;

if (!str_strcmp(bkend, &aaa_bkend_s))
if (str_match(bkend, &aaa_bkend_s))
return &aaa_leg_tags;

if (!str_strcmp(bkend, &evi_bkend_s))
if (str_match(bkend, &evi_bkend_s))
return &evi_leg_tags;

return NULL;
Expand Down
6 changes: 2 additions & 4 deletions modules/acc/acc_logic.c
Expand Up @@ -380,13 +380,11 @@ int w_acc_db_request(struct sip_msg *rq, str* comment, str *table)
env_set_comment( &accp );
env_set_text(table->s, table->len);

if (str_strcmp(table, &db_table_mc) == 0) {
if (str_match(table, &db_table_mc))
return acc_db_request(rq, NULL, &mc_ins_list, 0);
}

if (str_strcmp(table, &db_table_acc) == 0) {
if (str_match(table, &db_table_acc))
return acc_db_request(rq, NULL, &acc_ins_list, 0);
}

return acc_db_request( rq, NULL,NULL, 0);
}
Expand Down
2 changes: 1 addition & 1 deletion modules/dialog/dlg_handlers.c
Expand Up @@ -1360,7 +1360,7 @@ static inline int dlg_update_sdp(struct dlg_cell *dlg, struct sip_msg *msg,
return 0; /* nothing to do, no body */

/* check if we need to update it */
if (!str_strcmp(&dlg->legs[leg].in_sdp, &sdp)) {
if (str_match(&dlg->legs[leg].in_sdp, &sdp)) {
LM_DBG("SDP not changed, using the same one!\n");
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions modules/json/json.c
Expand Up @@ -892,9 +892,9 @@ int get_value(int state, json_name * id, char *start, char * cur)

break;
case ST_ITER:
if (!str_strcmp(&keys_s, &in))
if (str_match(&keys_s, &in))
id->iter_type = ITER_KEYS;
else if (!str_strcmp(&values_s, &in))
else if (str_match(&values_s, &in))
id->iter_type = ITER_VALUES;
else {
LM_ERR("Bad iterator type\n");
Expand Down
4 changes: 2 additions & 2 deletions modules/mid_registrar/save.c
Expand Up @@ -948,11 +948,11 @@ static contact_t *match_contact(ucontact_id ctid, struct sip_msg *msg)
continue;
}

if (!str_strcmp(&ctid_str, &puri.u_val[idx]))
if (str_match(&ctid_str, &puri.u_val[idx]))
return c;

} else {
if (!str_strcmp(&ctid_str, &puri.user))
if (str_match(&ctid_str, &puri.user))
return c;
}
}
Expand Down
7 changes: 3 additions & 4 deletions modules/registrar/lookup.c
Expand Up @@ -687,7 +687,7 @@ int is_contact_registered(struct sip_msg* _m, void *_d, str* _a,
LM_DBG("found AoR, searching for ct: '%.*s'\n", curi.len, curi.s);

for (c=r->contacts; c; c=c->next) {
if (!str_strcmp(&curi, &c->c))
if (str_match(&curi, &c->c))
goto out_found_unlock;
}
/* contact not defined; callid defined */
Expand All @@ -696,7 +696,7 @@ int is_contact_registered(struct sip_msg* _m, void *_d, str* _a,
callid.len, callid.s);

for (c=r->contacts; c; c=c->next) {
if (!str_strcmp(&callid, &c->callid))
if (str_match(&callid, &c->callid))
goto out_found_unlock;
}
/* both callid and contact defined */
Expand All @@ -705,8 +705,7 @@ int is_contact_registered(struct sip_msg* _m, void *_d, str* _a,
curi.len, curi.s, callid.len, callid.s);

for (c=r->contacts; c; c=c->next) {
if (!str_strcmp(&curi, &c->c) &&
!str_strcmp(&callid, &c->callid))
if (str_match(&curi, &c->c) && str_match(&callid, &c->callid))
goto out_found_unlock;
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/statistics/statistics.c
Expand Up @@ -277,7 +277,7 @@ static int fixup_iter_param(void **param)
list_for_each(ele, &script_iters) {
iter = list_entry(ele, struct stat_iter, list);

if (str_strcmp((str*)*param, &iter->name) == 0) {
if (str_match((str *)*param, &iter->name)) {
*param = iter;
return 0;
}
Expand Down

0 comments on commit d004a6c

Please sign in to comment.