Skip to content

Commit a1b2c06

Browse files
chu11chu11
committed
kvs: modernize cleanup saving of errno
Problem: Throughout the KVS code errnos are preserved in cleanup paths in a manner that is not common to the rest of the flux-core codebase. Cleanup these paths by either: A) Preserve errno within appropriate "destroy" functions. B) Use ERRNO_SAFE_WRAP
1 parent f13e2e6 commit a1b2c06

File tree

9 files changed

+85
-172
lines changed

9 files changed

+85
-172
lines changed

‎src/modules/kvs/kvs.c‎

Lines changed: 18 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "src/common/libutil/tstat.h"
3131
#include "src/common/libutil/timestamp.h"
3232
#include "src/common/libutil/errprintf.h"
33+
#include "src/common/libutil/errno_safe.h"
3334
#include "src/common/libkvs/treeobj.h"
3435
#include "src/common/libkvs/kvs_checkpoint.h"
3536
#include "src/common/libkvs/kvs_txn_private.h"
@@ -328,7 +329,6 @@ static void getroot_completion (flux_future_t *f, void *arg)
328329
uint32_t owner;
329330
const char *ref;
330331
struct kvsroot *root;
331-
int save_errno;
332332

333333
msg = flux_future_aux_get (f, "msg");
334334
assert (msg);
@@ -361,7 +361,7 @@ static void getroot_completion (flux_future_t *f, void *arg)
361361
}
362362

363363
if (event_subscribe (ctx, ns) < 0) {
364-
save_errno = errno;
364+
int save_errno = errno;
365365
kvsroot_mgr_remove_root (ctx->krm, ns);
366366
errno = save_errno;
367367
flux_log_error (ctx->h, "%s: event_subscribe", __FUNCTION__);
@@ -805,14 +805,13 @@ static int setroot_event_send (struct kvs_ctx *ctx,
805805
{
806806
flux_msg_t *msg = NULL;
807807
char *setroot_topic = NULL;
808-
int saved_errno, rc = -1;
808+
int rc = -1;
809809

810810
assert (ctx->rank == 0);
811811

812812
if (asprintf (&setroot_topic,
813813
"kvs.namespace-%s-setroot",
814814
root->ns_name) < 0) {
815-
saved_errno = errno;
816815
flux_log_error (ctx->h, "%s: asprintf", __FUNCTION__);
817816
goto done;
818817
}
@@ -825,24 +824,17 @@ static int setroot_event_send (struct kvs_ctx *ctx,
825824
"names", names,
826825
"keys", keys,
827826
"owner", root->owner))) {
828-
saved_errno = errno;
829827
flux_log_error (ctx->h, "%s: flux_event_pack", __FUNCTION__);
830828
goto done;
831829
}
832-
if (flux_msg_set_private (msg) < 0) {
833-
saved_errno = errno;
830+
if (flux_msg_set_private (msg) < 0)
834831
goto done;
835-
}
836-
if (flux_send (ctx->h, msg, 0) < 0) {
837-
saved_errno = errno;
832+
if (flux_send (ctx->h, msg, 0) < 0)
838833
goto done;
839-
}
840834
rc = 0;
841835
done:
842-
free (setroot_topic);
836+
ERRNO_SAFE_WRAP (free, setroot_topic);
843837
flux_msg_destroy (msg);
844-
if (rc < 0)
845-
errno = saved_errno;
846838
return rc;
847839
}
848840

@@ -853,10 +845,9 @@ static int error_event_send (struct kvs_ctx *ctx,
853845
{
854846
flux_msg_t *msg = NULL;
855847
char *error_topic = NULL;
856-
int saved_errno, rc = -1;
848+
int rc = -1;
857849

858850
if (asprintf (&error_topic, "kvs.namespace-%s-error", ns) < 0) {
859-
saved_errno = errno;
860851
flux_log_error (ctx->h, "%s: asprintf", __FUNCTION__);
861852
goto done;
862853
}
@@ -866,24 +857,17 @@ static int error_event_send (struct kvs_ctx *ctx,
866857
"namespace", ns,
867858
"names", names,
868859
"errnum", errnum))) {
869-
saved_errno = errno;
870860
flux_log_error (ctx->h, "%s: flux_event_pack", __FUNCTION__);
871861
goto done;
872862
}
873-
if (flux_msg_set_private (msg) < 0) {
874-
saved_errno = errno;
863+
if (flux_msg_set_private (msg) < 0)
875864
goto done;
876-
}
877-
if (flux_send (ctx->h, msg, 0) < 0) {
878-
saved_errno = errno;
865+
if (flux_send (ctx->h, msg, 0) < 0)
879866
goto done;
880-
}
881867
rc = 0;
882868
done:
883-
free (error_topic);
869+
ERRNO_SAFE_WRAP (free, error_topic);
884870
flux_msg_destroy (msg);
885-
if (rc < 0)
886-
errno = saved_errno;
887871
return rc;
888872
}
889873

@@ -2367,7 +2351,7 @@ static void start_root_remove (struct kvs_ctx *ctx, const char *ns)
23672351
static int namespace_remove (struct kvs_ctx *ctx, const char *ns)
23682352
{
23692353
flux_msg_t *msg = NULL;
2370-
int saved_errno, rc = -1;
2354+
int rc = -1;
23712355
char *topic = NULL;
23722356

23732357
/* Namespace doesn't exist or is already in process of being
@@ -2377,32 +2361,23 @@ static int namespace_remove (struct kvs_ctx *ctx, const char *ns)
23772361
goto done;
23782362
}
23792363

2380-
if (asprintf (&topic, "kvs.namespace-%s-removed", ns) < 0) {
2381-
saved_errno = errno;
2364+
if (asprintf (&topic, "kvs.namespace-%s-removed", ns) < 0)
23822365
goto cleanup;
2383-
}
23842366
if (!(msg = flux_event_pack (topic, "{ s:s }", "namespace", ns))) {
2385-
saved_errno = errno;
23862367
flux_log_error (ctx->h, "%s: flux_event_pack", __FUNCTION__);
23872368
goto cleanup;
23882369
}
2389-
if (flux_msg_set_private (msg) < 0) {
2390-
saved_errno = errno;
2370+
if (flux_msg_set_private (msg) < 0)
23912371
goto cleanup;
2392-
}
2393-
if (flux_send (ctx->h, msg, 0) < 0) {
2394-
saved_errno = errno;
2372+
if (flux_send (ctx->h, msg, 0) < 0)
23952373
goto cleanup;
2396-
}
23972374

23982375
start_root_remove (ctx, ns);
23992376
done:
24002377
rc = 0;
24012378
cleanup:
24022379
flux_msg_destroy (msg);
2403-
free (topic);
2404-
if (rc < 0)
2405-
errno = saved_errno;
2380+
ERRNO_SAFE_WRAP (free, topic);
24062381
return rc;
24072382
}
24082383

@@ -2932,12 +2907,11 @@ static int store_initial_rootdir (struct kvs_ctx *ctx, char *ref, int ref_len)
29322907
saved_errno = errno;
29332908
ret = cache_remove_entry (ctx->cache, ref);
29342909
assert (ret == 1);
2910+
errno = saved_errno;
29352911
error:
2936-
saved_errno = errno;
2937-
free (data);
2912+
ERRNO_SAFE_WRAP (free, data);
29382913
flux_future_destroy (f);
2939-
json_decref (rootdir);
2940-
errno = saved_errno;
2914+
ERRNO_SAFE_WRAP (json_decref, rootdir);
29412915
return -1;
29422916
}
29432917

‎src/modules/kvs/kvs_wait_version.c‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <flux/core.h>
2222

2323
#include "src/common/libczmqcontainers/czmq_containers.h"
24+
#include "src/common/libutil/errno_safe.h"
2425

2526
#include "kvs_wait_version.h"
2627

@@ -122,22 +123,21 @@ int kvs_wait_version_remove_msg (struct kvsroot *root,
122123
zlist_t *tmp = NULL;
123124
struct kvs_wait_version *kwv;
124125
int rc = -1;
125-
int saved_errno;
126126

127127
if (!root || !cmp) {
128-
saved_errno = EINVAL;
129-
goto error;
128+
errno = EINVAL;
129+
return -1;
130130
}
131131

132132
kwv = zlist_first (root->wait_version_list);
133133
while (kwv) {
134134
if (cmp (kwv->msg, arg)) {
135135
if (!tmp && !(tmp = zlist_new ())) {
136-
saved_errno = ENOMEM;
136+
errno = ENOMEM;
137137
goto error;
138138
}
139139
if (zlist_append (tmp, kwv) < 0) {
140-
saved_errno = ENOMEM;
140+
errno = ENOMEM;
141141
goto error;
142142
}
143143
}
@@ -154,9 +154,7 @@ int kvs_wait_version_remove_msg (struct kvsroot *root,
154154
* the original queue yet. Allow user to handle error as they see
155155
* fit.
156156
*/
157-
zlist_destroy (&tmp);
158-
if (rc < 0)
159-
errno = saved_errno;
157+
ERRNO_SAFE_WRAP (zlist_destroy, &tmp);
160158
return rc;
161159
}
162160

‎src/modules/kvs/kvsroot.c‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,15 @@ struct kvsroot_mgr {
3939
kvsroot_mgr_t *kvsroot_mgr_create (flux_t *h, void *arg)
4040
{
4141
kvsroot_mgr_t *krm = NULL;
42-
int saved_errno;
4342

44-
if (!(krm = calloc (1, sizeof (*krm)))) {
45-
saved_errno = ENOMEM;
43+
if (!(krm = calloc (1, sizeof (*krm))))
4644
goto error;
47-
}
4845
if (!(krm->roothash = zhash_new ())) {
49-
saved_errno = ENOMEM;
46+
errno = ENOMEM;
5047
goto error;
5148
}
5249
if (!(krm->removelist = zlist_new ())) {
53-
saved_errno = ENOMEM;
50+
errno = ENOMEM;
5451
goto error;
5552
}
5653
krm->iterating_roots = false;
@@ -60,18 +57,19 @@ kvsroot_mgr_t *kvsroot_mgr_create (flux_t *h, void *arg)
6057

6158
error:
6259
kvsroot_mgr_destroy (krm);
63-
errno = saved_errno;
6460
return NULL;
6561
}
6662

6763
void kvsroot_mgr_destroy (kvsroot_mgr_t *krm)
6864
{
6965
if (krm) {
66+
int save_errno = errno;
7067
if (krm->roothash)
7168
zhash_destroy (&krm->roothash);
7269
if (krm->removelist)
7370
zlist_destroy (&krm->removelist);
7471
free (krm);
72+
errno = save_errno;
7573
}
7674
}
7775

0 commit comments

Comments
 (0)