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)
    (cherry picked from commit d004a6c)
  • Loading branch information
@liviuchircu
liviuchircu committed Nov 4, 2020
1 parent 8c207e9 commit 1525893
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 57 deletions.
2 changes: 1 addition & 1 deletion cachedb/cachedb_dict.c
View file
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
View file
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
View file
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
4 changes: 2 additions & 2 deletions modules/mid_registrar/save.c
View file
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
View file
Expand Up @@ -718,7 +718,7 @@ int is_contact_registered(struct sip_msg* _m, char *_d, char* _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 @@ -727,7 +727,7 @@ int is_contact_registered(struct sip_msg* _m, char *_d, char* _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 @@ -736,8 +736,7 @@ int is_contact_registered(struct sip_msg* _m, char *_d, char* _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
View file
Expand Up @@ -296,7 +296,7 @@ static int _fixup_iter_param(void **param)

name.s = *param;
name.len = strlen(name.s);
if (str_strcmp(&name, &iter->name) == 0) {
if (str_match(&name, &iter->name)) {
*param = &iter->cur;
return 0;
}
Expand Down

0 comments on commit 1525893

Please sign in to comment.