I'm sending the last wave of patches cleaning dead assignments or fixing code
around them.
0001-Dead-assignments-cleanup-in-providers-code.patch:
Here the biggest change is in prototype of sdap_access_decide_offline, which
IMO
doesn't need to return any value, since it returns alway EOK now and there is
no code which could be failing.
0002-Dead-assignments-cleanup-in-NSS-responder.patch
0003-Dead-assignments-cleanup-in-memberof-module.patch:
Here I'm not entirely sure about return code LDB_ERR_OPERATIONS_ERROR. The
variable ret can contain either 0 or -1 (in case of insufficient memory, I
checked the ldb code). I think returning -1 further isn't the right choice,
thus either ENOMEM or LDB_ERR_OPERATIONS_ERR seem to be acceptable. I chose
the latter one, because it indicates generic ldb error. ENOMEM could be
convenient now, but in the future the behavior of ldb_schema_attribute_add
might change and ENOMEM would become misleading.
0004-Dead-assignments-cleanup-in-various-places-in-SSSD.patch
The last hunk should be fixing this: http://jhrozek.fedorapeople.org/sssd-
clang-llvm/report-G4QCn7.html#EndPath . IMO the original code had a flaw there,
which would make the inner while cycle useless in case write() actually wrote
less bytes than it could. This way it should be fixed.
--
Jan
From f1727032bcf4b8e0bb147a84d07e531c17429877 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Wed, 1 Sep 2010 16:30:32 +0200
Subject: [PATCH 1/4] Dead assignments cleanup in providers code
Dead assignments were deleted. Also prototype of function
sdap_access_decide_offline() has been changed, since its return
code was never used.
Ticket: #586
---
src/providers/child_common.c | 2 --
src/providers/data_provider_opts.c | 1 -
src/providers/krb5/krb5_child.c | 1 -
src/providers/ldap/ldap_id_enum.c | 2 --
src/providers/ldap/sdap_access.c | 18 +++++++-----------
src/providers/proxy/proxy_id.c | 2 --
6 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/src/providers/child_common.c b/src/providers/child_common.c
index a242338..5b2251f 100644
--- a/src/providers/child_common.c
+++ b/src/providers/child_common.c
@@ -492,14 +492,12 @@ void child_cleanup(int readfd, int writefd)
if (readfd != -1) {
ret = close(readfd);
if (ret != EOK) {
- ret = errno;
DEBUG(1, ("close failed [%d][%s].\n", errno, strerror(errno)));
}
}
if (writefd != -1) {
ret = close(writefd);
if (ret != EOK) {
- ret = errno;
DEBUG(1, ("close failed [%d][%s].\n", errno, strerror(errno)));
}
}
diff --git a/src/providers/data_provider_opts.c b/src/providers/data_provider_opts.c
index 98283e4..14f48ac 100644
--- a/src/providers/data_provider_opts.c
+++ b/src/providers/data_provider_opts.c
@@ -42,7 +42,6 @@ int dp_get_options(TALLOC_CTX *memctx,
opts[i].opt_name = def_opts[i].opt_name;
opts[i].type = def_opts[i].type;
opts[i].def_val = def_opts[i].def_val;
- ret = EOK;
switch (def_opts[i].type) {
case DP_OPT_STRING:
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index cfb3f42..6892c66 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -270,7 +270,6 @@ static krb5_error_code create_ccache_file(struct krb5_req *kr, krb5_creds *creds
done:
if (fd != -1) {
close(fd);
- fd = -1;
}
if (kerr != 0 && tmp_cc != NULL) {
krb5_cc_destroy(kr->ctx, tmp_cc);
diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
index 87b9563..0917e71 100644
--- a/src/providers/ldap/ldap_id_enum.c
+++ b/src/providers/ldap/ldap_id_enum.c
@@ -531,8 +531,6 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
state->ctx = ctx;
state->op = op;
- attr_name = ctx->opts->group_map[SDAP_AT_GROUP_NAME].name;
-
if (ctx->max_group_timestamp && !purge) {
state->filter = talloc_asprintf(state,
diff --git a/src/providers/ldap/sdap_access.c b/src/providers/ldap/sdap_access.c
index 7d7832a..90b5371 100644
--- a/src/providers/ldap/sdap_access.c
+++ b/src/providers/ldap/sdap_access.c
@@ -99,7 +99,7 @@ struct sdap_access_req_ctx {
char *basedn;
};
-static int sdap_access_decide_offline(struct tevent_req *req);
+static void sdap_access_decide_offline(struct tevent_req *req);
static int sdap_access_retry(struct tevent_req *req);
static void sdap_access_connect_done(struct tevent_req *subreq);
static void sdap_access_get_access_done(struct tevent_req *req);
@@ -182,7 +182,7 @@ static struct tevent_req *sdap_access_send(TALLOC_CTX *mem_ctx,
/* Ok, we have one result, check if we are online or offline */
if (be_is_offline(state->be_ctx)) {
/* Ok, we're offline. Return from the cache */
- ret = sdap_access_decide_offline(req);
+ sdap_access_decide_offline(req);
goto finished;
}
@@ -241,7 +241,7 @@ finished:
return req;
}
-static int sdap_access_decide_offline(struct tevent_req *req)
+static void sdap_access_decide_offline(struct tevent_req *req)
{
struct sdap_access_req_ctx *state =
tevent_req_data(req, struct sdap_access_req_ctx);
@@ -253,8 +253,6 @@ static int sdap_access_decide_offline(struct tevent_req *req)
DEBUG(6, ("Access denied by cached credentials\n"));
state->pam_status = PAM_PERM_DENIED;
}
-
- return EOK;
}
static int sdap_access_retry(struct tevent_req *req)
@@ -287,11 +285,9 @@ static void sdap_access_connect_done(struct tevent_req *subreq)
if (ret != EOK) {
if (dp_error == DP_ERR_OFFLINE) {
- ret = sdap_access_decide_offline(req);
- if (ret == EOK) {
- tevent_req_done(req);
- return;
- }
+ sdap_access_decide_offline(req);
+ tevent_req_done(req);
+ return;
}
tevent_req_error(req, ret);
@@ -344,7 +340,7 @@ static void sdap_access_get_access_done(struct tevent_req *subreq)
}
state->pam_status = PAM_SYSTEM_ERR;
} else if (dp_error == DP_ERR_OFFLINE) {
- ret = sdap_access_decide_offline(req);
+ sdap_access_decide_offline(req);
} else {
DEBUG(1, ("sdap_get_generic_send() returned error [%d][%s]\n",
ret, strerror(ret)));
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index 8536b93..d2fba72 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -1037,7 +1037,6 @@ void proxy_get_account_info(struct be_req *breq)
{
struct be_acct_req *ar;
struct proxy_id_ctx *ctx;
- struct tevent_context *ev;
struct sysdb_ctx *sysdb;
struct sss_domain_info *domain;
uid_t uid;
@@ -1047,7 +1046,6 @@ void proxy_get_account_info(struct be_req *breq)
ar = talloc_get_type(breq->req_data, struct be_acct_req);
ctx = talloc_get_type(breq->be_ctx->bet_info[BET_ID].pvt_bet_data,
struct proxy_id_ctx);
- ev = breq->be_ctx->ev;
sysdb = breq->be_ctx->sysdb;
domain = breq->be_ctx->domain;
--
1.7.2.1
From fabd2f5eff81dcc2056693ab550b9fb1b22bd354 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Thu, 2 Sep 2010 10:07:33 +0200
Subject: [PATCH 2/4] Dead assignments cleanup in NSS responder
Various dead assignments were deleted, some return value inspections
were added.
Ticket: #588
---
src/responder/common/negcache.c | 2 --
src/responder/nss/nsssrv_cmd.c | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c
index 521a2e7..9ba24c8 100644
--- a/src/responder/common/negcache.c
+++ b/src/responder/common/negcache.c
@@ -395,7 +395,6 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache,
}
filter_list[1] = NULL;
}
- ret = EOK;
}
else if (ret != EOK) goto done;
@@ -486,7 +485,6 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache,
}
filter_list[1] = NULL;
}
- ret = EOK;
}
else if (ret != EOK) goto done;
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 9b75513..719608c 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -1272,6 +1272,9 @@ static int nss_cmd_endpwent(struct cli_ctx *cctx)
sss_packet_get_cmd(cctx->creq->in),
&cctx->creq->out);
+ if (ret != EOK) {
+ return ret;
+ }
if (nctx->pctx == NULL) goto done;
/* free results and reset */
@@ -2342,6 +2345,9 @@ static int nss_cmd_endgrent(struct cli_ctx *cctx)
sss_packet_get_cmd(cctx->creq->in),
&cctx->creq->out);
+ if (ret != EOK) {
+ return ret;
+ }
if (nctx->gctx == NULL) goto done;
/* free results and reset */
@@ -2395,11 +2401,7 @@ static int nss_cmd_initgr_send_reply(struct nss_dom_ctx *dctx)
{
struct nss_cmd_ctx *cmdctx = dctx->cmdctx;
struct cli_ctx *cctx = cmdctx->cctx;
- struct nss_ctx *nctx;
int ret;
- int i;
-
- nctx = talloc_get_type(cctx->rctx->pvt_ctx, struct nss_ctx);
ret = sss_packet_new(cctx->creq, 0,
sss_packet_get_cmd(cctx->creq->in),
@@ -2407,7 +2409,7 @@ static int nss_cmd_initgr_send_reply(struct nss_dom_ctx *dctx)
if (ret != EOK) {
return EFAULT;
}
- i = dctx->res->count;
+
ret = fill_initgr(cctx->creq->out, dctx->res);
if (ret) {
return ret;
--
1.7.2.1
From a31db46c664f85fefa4b44b17830a61d5bbca18e Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Thu, 2 Sep 2010 13:27:25 +0200
Subject: [PATCH 3/4] Dead assignments cleanup in memberof module
Some assignments deleted, two return value inspections were
added.
Ticket: #589
---
src/ldb_modules/memberof.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
index c3f5763..1e28593 100644
--- a/src/ldb_modules/memberof.c
+++ b/src/ldb_modules/memberof.c
@@ -1712,14 +1712,12 @@ static int mbof_del_execute_cont(struct mbof_del_operation *delop)
struct mbof_del_operation *parent;
struct mbof_del_ctx *del_ctx;
struct mbof_ctx *ctx;
- struct ldb_context *ldb;
struct mbof_dn_array *new_list;
int i;
del_ctx = delop->del_ctx;
parent = delop->parent;
ctx = del_ctx->ctx;
- ldb = ldb_module_get_ctx(ctx->module);
anc_ctx = talloc_zero(delop, struct mbof_del_ancestors_ctx);
if (!anc_ctx) {
@@ -2194,11 +2192,9 @@ static int mbof_del_get_next(struct mbof_del_operation *delop,
{
struct mbof_del_operation *top, *cop;
struct mbof_del_ctx *del_ctx;
- struct mbof_ctx *ctx;
struct mbof_dn *save, *tmp;
del_ctx = delop->del_ctx;
- ctx = del_ctx->ctx;
/* first of all, save the current delop in the history */
save = talloc_zero(del_ctx, struct mbof_dn);
@@ -2796,12 +2792,10 @@ static int mbof_mod_delete(struct mbof_mod_ctx *mod_ctx,
{
struct mbof_del_operation *first;
struct mbof_del_ctx *del_ctx;
- struct ldb_context *ldb;
struct mbof_ctx *ctx;
int i, ret;
ctx = mod_ctx->ctx;
- ldb = ldb_module_get_ctx(ctx->module);
del_ctx = talloc_zero(mod_ctx, struct mbof_del_ctx);
if (!del_ctx) {
@@ -3603,7 +3597,10 @@ static int memberof_init(struct ldb_module *module)
/* set syntaxes for member and memberof so that comparisons in filters and
* such are done right */
ret = ldb_schema_attribute_add(ldb, DB_MEMBER, 0, LDB_SYNTAX_DN);
+ if (ret != 0) return LDB_ERR_OPERATIONS_ERROR;
+
ret = ldb_schema_attribute_add(ldb, DB_MEMBEROF, 0, LDB_SYNTAX_DN);
+ if (ret != 0) return LDB_ERR_OPERATIONS_ERROR;
return ldb_next_init(module);
}
--
1.7.2.1
From 3f332302528452e2f18cbcabfc0f39efab1f0146 Mon Sep 17 00:00:00 2001
From: Jan Zeleny <jzel...@redhat.com>
Date: Thu, 2 Sep 2010 14:35:58 +0200
Subject: [PATCH 4/4] Dead assignments cleanup in various places in SSSD
Three assignments deleted, two return code inspection added.
Also found and fixed one critical bug caused by dead assignment.
Ticket: #590
---
src/confdb/confdb.c | 1 -
src/confdb/confdb_setup.c | 5 +----
src/db/sysdb.c | 3 +++
src/monitor/monitor.c | 3 +++
src/util/backup_file.c | 2 +-
5 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 877b80a..eebc2cd 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -862,7 +862,6 @@ int confdb_get_domains(struct confdb_ctx *cdb,
if (ret) {
DEBUG(0, ("Error (%d [%s]) retrieving domain [%s], skipping!\n",
ret, strerror(ret), domlist[i]));
- ret = EOK;
continue;
}
diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
index b2ac7e1..f98a697 100644
--- a/src/confdb/confdb_setup.c
+++ b/src/confdb/confdb_setup.c
@@ -133,7 +133,6 @@ static int confdb_create_ldif(TALLOC_CTX *mem_ctx,
int ret, i, j;
char *ldif;
char *tmp_ldif;
- char *writer;
char **sections;
int section_count;
char *dn;
@@ -159,7 +158,6 @@ static int confdb_create_ldif(TALLOC_CTX *mem_ctx,
}
memcpy(ldif, CONFDB_INTERNAL_LDIF, ldif_len);
- writer = ldif+ldif_len;
/* Read in the collection and convert it to an LDIF */
/* Get the list of sections */
@@ -273,7 +271,7 @@ error:
int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
{
- int ret, i;
+ int ret;
int fd = -1;
struct collection_item *sssd_config = NULL;
struct collection_item *error_list = NULL;
@@ -397,7 +395,6 @@ int confdb_init_db(const char *config_file, struct confdb_ctx *cdb)
DEBUG(7, ("LDIF file to import: \n%s", config_ldif));
- i=0;
while ((ldif = ldb_ldif_read_string(cdb->ldb, (const char **)&config_ldif))) {
ret = ldb_add(cdb->ldb, ldif->msg);
if (ret != LDB_SUCCESS) {
diff --git a/src/db/sysdb.c b/src/db/sysdb.c
index 5a7626f..4e1243c 100644
--- a/src/db/sysdb.c
+++ b/src/db/sysdb.c
@@ -193,6 +193,9 @@ int sysdb_attrs_add_val(struct sysdb_attrs *attrs,
int ret;
ret = sysdb_attrs_get_el(attrs, name, &el);
+ if (ret != EOK) {
+ return ret;
+ }
vals = talloc_realloc(attrs->a, el->values,
struct ldb_val, el->num_values+1);
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index caf4056..1c2a058 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -981,6 +981,9 @@ static int add_new_service(struct mt_ctx *ctx, const char *name)
struct mt_svc *svc;
ret = get_service_config(ctx, name, &svc);
+ if (ret != EOK) {
+ return ret;
+ }
ret = start_service(svc);
if (ret != EOK) {
diff --git a/src/util/backup_file.c b/src/util/backup_file.c
index 9907932..fb1604b 100644
--- a/src/util/backup_file.c
+++ b/src/util/backup_file.c
@@ -96,8 +96,8 @@ int backup_file(const char *src_file, int dbglvl)
count = num;
+ pos = 0;
while (count > 0) {
- pos = 0;
errno = 0;
num = write(dst_fd, &buf[pos], count);
if (num < 0) {
--
1.7.2.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
