From 1525893e2e3ee2002d48de0fb42284affae4acf1 Mon Sep 17 00:00:00 2001 From: Liviu Chircu Date: Wed, 4 Nov 2020 15:34:41 +0200 Subject: [PATCH] Code quality: Use str_match() instead of !str_strcmp() 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 765a84b293db8ec3da960c0ae61f725f3519bfd9) (cherry picked from commit d004a6c5e35b7dac783b52dd95fa11d6ad2d13c2) --- cachedb/cachedb_dict.c | 2 +- lib/test/test_csv.c | 82 ++++++++++++++++----------------- modules/acc/acc_extra.c | 16 +++---- modules/mid_registrar/save.c | 4 +- modules/registrar/lookup.c | 7 ++- modules/statistics/statistics.c | 2 +- 6 files changed, 56 insertions(+), 57 deletions(-) diff --git a/cachedb/cachedb_dict.c b/cachedb/cachedb_dict.c index 43db0b8e8e6..520964b8f2c 100644 --- a/cachedb/cachedb_dict.c +++ b/cachedb/cachedb_dict.c @@ -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; } diff --git a/lib/test/test_csv.c b/lib/test/test_csv.c index a98983759c4..b49dfc02ab1 100644 --- a/lib/test/test_csv.c +++ b/lib/test/test_csv.c @@ -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); } @@ -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); @@ -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); @@ -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); } diff --git a/modules/acc/acc_extra.c b/modules/acc/acc_extra.c index 79a64e3c5bf..05f7bb6617a 100644 --- a/modules/acc/acc_extra.c +++ b/modules/acc/acc_extra.c @@ -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; @@ -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; diff --git a/modules/mid_registrar/save.c b/modules/mid_registrar/save.c index 90c61d10e20..fd39f313b2a 100644 --- a/modules/mid_registrar/save.c +++ b/modules/mid_registrar/save.c @@ -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; } } diff --git a/modules/registrar/lookup.c b/modules/registrar/lookup.c index efa12482785..7d0b15649c9 100644 --- a/modules/registrar/lookup.c +++ b/modules/registrar/lookup.c @@ -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 */ @@ -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 */ @@ -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; } } diff --git a/modules/statistics/statistics.c b/modules/statistics/statistics.c index 906d5430301..720b84b8ac8 100644 --- a/modules/statistics/statistics.c +++ b/modules/statistics/statistics.c @@ -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; }