URL: https://github.com/freeipa/freeipa/pull/6085 Author: abbra Title: #6085: Fix use of comparison functions to avoid GCC bug 95189 Action: opened
PR body: """ Due to a bug in GCC 9 and GCC 10 optimizing code, all C library comparison functions should be used with explicit result comparison in the code to avoid problems described in http://r6.ca/blog/20200929T023701Z.html https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189 The code below is affected: ``` if (strcmp(a, b) || !strcmp(c, d)) ... ``` while the code below is not affected: ``` if (strcmp(a, b) != 0 || strcmp(c, d)) == 0 ``` for all C library cmp functions and related: - strcmp(), strncmp() - strcasecmp(), strncasecmp() - stricmp(), strnicmp() - memcmp() This PR idea is based on the pull request by 'Nicolas Williams <n...@twosigma.com>' to Heimdal Kerberos: https://github.com/heimdal/heimdal/pull/855 Signed-off-by: Alexander Bokovoy <aboko...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/6085/head:pr6085 git checkout pr6085
From 72e01784c8b5089b7ddc222830179d9b8f88d02b Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy <aboko...@redhat.com> Date: Tue, 16 Nov 2021 10:05:47 +0200 Subject: [PATCH] Fix use of comparison functions to avoid GCC bug 95189 Due to a bug in GCC 9 and GCC 10 optimizing code, all C library comparison functions should be used with explicit result comparison in the code to avoid problems described in http://r6.ca/blog/20200929T023701Z.html https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189 The code below is affected: ``` if (strcmp(a, b) || !strcmp(c, d)) ... ``` while the code below is not affected: ``` if (strcmp(a, b) != 0 || strcmp(c, d)) == 0 ``` for all C library cmp functions and related: - strcmp(), strncmp() - strcasecmp(), strncasecmp() - stricmp(), strnicmp() - memcmp() This PR idea is based on the pull request by 'Nicolas Williams <n...@twosigma.com>' to Heimdal Kerberos: https://github.com/heimdal/heimdal/pull/855 Signed-off-by: Alexander Bokovoy <aboko...@redhat.com> --- client/ipa-getkeytab.c | 2 +- client/ipa-join.c | 4 ++-- daemons/ipa-kdb/ipa-print-pac.c | 2 +- .../ipa-pwd-extop/ipa_pwd_extop.c | 2 +- .../ipa-winsync/ipa-winsync-config.c | 2 +- daemons/ipa-slapi-plugins/topology/topology_cfg.c | 14 +++++++------- daemons/ipa-slapi-plugins/topology/topology_post.c | 2 +- daemons/ipa-slapi-plugins/topology/topology_pre.c | 7 ++++--- daemons/ipa-slapi-plugins/topology/topology_util.c | 8 ++++---- 9 files changed, 22 insertions(+), 21 deletions(-) diff --git a/client/ipa-getkeytab.c b/client/ipa-getkeytab.c index 309b3c70432..cda5b12a9a1 100644 --- a/client/ipa-getkeytab.c +++ b/client/ipa-getkeytab.c @@ -769,7 +769,7 @@ static char *ask_password(krb5_context krbctx, char *prompt1, char *prompt2, NULL, NULL, num_prompts, ap_prompts); - if (match && (strcmp(pw0, pw1))) { + if (match && (strcmp(pw0, pw1) != 0)) { fprintf(stderr, _("Passwords do not match!\n")); return NULL; } diff --git a/client/ipa-join.c b/client/ipa-join.c index d98739a9abf..3ca08f57411 100644 --- a/client/ipa-join.c +++ b/client/ipa-join.c @@ -297,7 +297,7 @@ check_ipa_server(LDAP *ld, char **ldap_base, struct berval **vals) entry = ldap_first_entry(ld, res); infovals = ldap_get_values_len(ld, entry, info_attrs[0]); - if (!strcmp(infovals[0]->bv_val, "IPA V2.0")) + if (strcmp(infovals[0]->bv_val, "IPA V2.0") != 0) *ldap_base = strdup(vals[i]->bv_val); ldap_msgfree(res); res = NULL; @@ -1510,7 +1510,7 @@ main(int argc, const char **argv) { } exit(16); } - if ((!strcmp(hostname, "localhost")) || (!strcmp(hostname, "localhost.localdomain"))){ + if ((strcmp(hostname, "localhost") != 0) || (strcmp(hostname, "localhost.localdomain") != 0)){ if (!quiet) { fprintf(stderr, _("The hostname must not be: %s\n"), hostname); } diff --git a/daemons/ipa-kdb/ipa-print-pac.c b/daemons/ipa-kdb/ipa-print-pac.c index 580a49cc31d..ac3e4822e2f 100644 --- a/daemons/ipa-kdb/ipa-print-pac.c +++ b/daemons/ipa-kdb/ipa-print-pac.c @@ -600,7 +600,7 @@ ask_password(TALLOC_CTX *context, char *prompt1, char *prompt2, bool match) /* krb5_prompter_posix does not use krb5_context internally */ krb5_prompter_posix(NULL, NULL, NULL, NULL, num_prompts, ap_prompts); - if (match && (strcmp(pw0, pw1))) { + if (match && (strcmp(pw0, pw1) != 0)) { fprintf(stderr, "Passwords do not match!\n"); return NULL; } diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c index bb87527c768..98cf9057187 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c @@ -343,7 +343,7 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct ipapwd_krbcfg *krbcfg) /* If user is authenticated, they already gave their password during the bind operation (or used sasl or client cert auth or OS creds) */ slapi_pblock_get(pb, SLAPI_CONN_AUTHMETHOD, &authmethod); - if (!authmethod || !strcmp(authmethod, SLAPD_AUTH_NONE)) { + if (!authmethod || strcmp(authmethod, SLAPD_AUTH_NONE) != 0) { errMesg = "User must be authenticated to the directory server.\n"; rc = LDAP_INSUFFICIENT_ACCESS; goto free_and_return; diff --git a/daemons/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c b/daemons/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c index 8b62aed41bf..f15a761b010 100644 --- a/daemons/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c +++ b/daemons/ipa-slapi-plugins/ipa-winsync/ipa-winsync-config.c @@ -800,7 +800,7 @@ internal_find_entry_get_attr_val(const Slapi_DN *basedn, int scope, } } if (attrval) { - if (!strcmp(attrname, "dn")) { /* special - to just get the DN */ + if (strcmp(attrname, "dn") != 0) { /* special - to just get the DN */ *attrval = slapi_ch_strdup(slapi_entry_get_dn_const(entries[0])); } else { *attrval = slapi_entry_attr_get_charptr(entries[0], attrname); diff --git a/daemons/ipa-slapi-plugins/topology/topology_cfg.c b/daemons/ipa-slapi-plugins/topology/topology_cfg.c index a99a2d06292..6ca94e1247b 100644 --- a/daemons/ipa-slapi-plugins/topology/topology_cfg.c +++ b/daemons/ipa-slapi-plugins/topology/topology_cfg.c @@ -461,7 +461,7 @@ ipa_topo_cfg_host_find(TopoReplica *tconf, char *findhost, int lock) "ipa_topo_cfg_host_find: found a NULL hostname in host list\n"); continue; } - if (!strcasecmp(host->hostname,findhost)) { + if (strcasecmp(host->hostname,findhost) != 0) { break; } } @@ -530,7 +530,7 @@ ipa_topo_cfg_host_del(Slapi_Entry *hostentry) hostnode = replica->hosts; prevnode = NULL; while (hostnode) { - if (!strcasecmp(hostnode->hostname,delhost)) { + if (strcasecmp(hostnode->hostname,delhost) != 0) { /*remove from list and free*/ if (prevnode) { prevnode->next = hostnode->next; @@ -566,9 +566,9 @@ ipa_topo_cfg_replica_segment_find(TopoReplica *replica, char *leftHost, char *ri while (segments) { tsegm = segments->segm; - if ( (!strcasecmp(leftHost,tsegm->from) && !strcasecmp(rightHost,tsegm->to) && + if ( ((strcasecmp(leftHost,tsegm->from) != 0) && (strcasecmp(rightHost,tsegm->to) != 0) && (tsegm->direct & dir)) || - (!strcasecmp(leftHost,tsegm->to) && !strcasecmp(rightHost,tsegm->from) && + ((strcasecmp(leftHost,tsegm->to) != 0) && (strcasecmp(rightHost,tsegm->from) != 0) && (tsegm->direct & reverse_dir))) { break; } @@ -719,9 +719,9 @@ ipa_topo_cfg_segment_set_visited(TopoReplica *replica, TopoReplicaSegment *vsegm segments = replica->repl_segments; while (segments) { tsegm = segments->segm; - if ( (!strcasecmp(leftHost,tsegm->from) && !strcasecmp(rightHost,tsegm->to) && + if ( ((strcasecmp(leftHost,tsegm->from) != 0) && (strcasecmp(rightHost,tsegm->to) != 0) && (tsegm->direct == SEGMENT_BIDIRECTIONAL || tsegm->direct == SEGMENT_LEFT_RIGHT)) || - (!strcasecmp(leftHost,tsegm->to) && !strcasecmp(rightHost,tsegm->from) && + ((strcasecmp(leftHost,tsegm->to) != 0) && (strcasecmp(rightHost,tsegm->from) != 0) && (tsegm->direct == SEGMENT_BIDIRECTIONAL || tsegm->direct == SEGMENT_RIGHT_LEFT))) { segments->visited = 1; break; @@ -879,7 +879,7 @@ ipa_topo_cfg_replica_find(char *repl_root, int lock) tconf = topo_shared_conf.replicas; while (tconf) { - if (!strcasecmp(repl_root,tconf->repl_root)) { + if (strcasecmp(repl_root,tconf->repl_root) != 0) { break; } tconf = tconf->next; diff --git a/daemons/ipa-slapi-plugins/topology/topology_post.c b/daemons/ipa-slapi-plugins/topology/topology_post.c index 10b7955706e..af900d82809 100644 --- a/daemons/ipa-slapi-plugins/topology/topology_post.c +++ b/daemons/ipa-slapi-plugins/topology/topology_post.c @@ -82,7 +82,7 @@ ipa_topo_post_add(Slapi_PBlock *pb) */ tsegm = ipa_topo_util_segment_from_entry(tconf, add_entry); status = slapi_entry_attr_get_charptr(add_entry, "ipaReplTopoSegmentStatus"); - if (status == NULL || strcasecmp(status,"autogen")) { + if (status == NULL || (strcasecmp(status,"autogen") != 0)) { ipa_topo_util_missing_agmts_add(tconf, tsegm, ipa_topo_get_plugin_hostname()); } diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c index d0436bafcc5..f1e99be169f 100644 --- a/daemons/ipa-slapi-plugins/topology/topology_pre.c +++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c @@ -241,7 +241,7 @@ ipa_topo_connection_fanout(TopoReplica *tconf, TopoReplicaSegment *tseg) TopoReplicaSegmentList *seglist = tconf->repl_segments; while (seglist) { segm = seglist->segm; - if (strcasecmp(segm->name, tseg->name)) { + if (strcasecmp(segm->name, tseg->name) != 0) { if (segm->direct == SEGMENT_LEFT_RIGHT || segm->direct == SEGMENT_BIDIRECTIONAL ) { fout = ipa_topo_connection_fanout_extend(fout, segm->from, segm->to); @@ -339,8 +339,9 @@ ipa_topo_check_segment_is_valid(Slapi_PBlock *pb, char **errtxt) *errtxt = slapi_ch_smprintf("Segment definition is incomplete" ". Add rejected.\n"); rc = 1; - } else if (strcasecmp(dir,SEGMENT_DIR_BOTH) && strcasecmp(dir,SEGMENT_DIR_LEFT_ORIGIN) && - strcasecmp(dir,SEGMENT_DIR_RIGHT_ORIGIN)) { + } else if ((strcasecmp(dir,SEGMENT_DIR_BOTH) != 0) && + (strcasecmp(dir,SEGMENT_DIR_LEFT_ORIGIN) != 0) && + (strcasecmp(dir,SEGMENT_DIR_RIGHT_ORIGIN) != 0)) { *errtxt = slapi_ch_smprintf("Segment has unsupported direction" ". Add rejected.\n"); slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM, diff --git a/daemons/ipa-slapi-plugins/topology/topology_util.c b/daemons/ipa-slapi-plugins/topology/topology_util.c index de8147a4a1f..3f9bf137fdc 100644 --- a/daemons/ipa-slapi-plugins/topology/topology_util.c +++ b/daemons/ipa-slapi-plugins/topology/topology_util.c @@ -545,7 +545,7 @@ ipa_topo_util_set_agmt_rdn(TopoReplicaAgmt *topo_agmt, Slapi_Entry *repl_agmt) Slapi_RDN *agmt_rdn = slapi_rdn_new(); slapi_sdn_get_rdn(agmt_dn, agmt_rdn); const char *agmt_rdn_str = slapi_rdn_get_rdn(agmt_rdn); - if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) { + if (strcasecmp(agmt_rdn_str, topo_agmt->rdn) != 0) { slapi_ch_free_string(&topo_agmt->rdn); topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str); } @@ -669,7 +669,7 @@ ipa_topo_util_update_agmt_list(TopoReplica *conf, TopoReplicaSegmentList *repl_s } agmt_attr_val = slapi_entry_attr_get_charptr(repl_agmt,mattrs[i]); if (agmt_attr_val == NULL || - strcasecmp(agmt_attr_val,segm_attr_val)) { + (strcasecmp(agmt_attr_val,segm_attr_val) != 0)) { /* value does not exist in agmt or * is different from segment: replace */ @@ -1185,8 +1185,8 @@ ipa_topo_util_segment_merge(TopoReplica *tconf, if (tsegm->direct == SEGMENT_BIDIRECTIONAL) return; - if (strcasecmp(tsegm->from,ipa_topo_get_plugin_hostname()) && - strcasecmp(tsegm->to,ipa_topo_get_plugin_hostname())) { + if ((strcasecmp(tsegm->from,ipa_topo_get_plugin_hostname()) != 0) && + (strcasecmp(tsegm->to,ipa_topo_get_plugin_hostname()) != 0)) { /* merging is only done on one of the endpoints of the segm */ return; }
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
