From b32ee30a095b08f4b2429c1f182c8715e2927220 Mon Sep 17 00:00:00 2001 From: pjd Date: Wed, 23 Jun 2010 23:07:57 +0000 Subject: [PATCH] MFC r209263: r209175: Eliminate dead code. Found by: Coverity Prevent CID: 5158 r209177: Remove macros that are not really needed. The idea was to have them in case we grow more descriptors, but I'll reconsider readding them once we get there. Passing (a = b) expression to FD_ISSET() is bad idea, as FD_ISSET() evaluates its argument twice. Found by: Coverity Prevent CID: 5243 r209179: Plug memory leaks. Found by: Coverity Prevent CID: 7052, 7053, 7054, 7055 r209180: Plug memory leak. Found by: Coverity Prevent CID: 7051 r209181: Plug memory leak. Found by: Coverity Prevent CID: 7056 r209182: Plug memory leak. Found by: Coverity Prevent CID: 7057 r209183: Initialize gctl_seq for synchronization requests. Reported by: hiroshi@soupacific.com Analysed by: Mikolaj Golub Tested by: hiroshi@soupacific.com, Mikolaj Golub r209184: Fix typos. r209185: Correct various log messages. Submitted by: Mikolaj Golub Note that without some of these changes hastd won't work on 8.x properly. Approved by: re (kensmith) git-svn-id: svn://svn.freebsd.org/base/releng/8.1@209488 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sbin/hastd/ebuf.c | 14 +++++++------- sbin/hastd/hast_proto.c | 4 +--- sbin/hastd/hastd.c | 36 ++++++++++++++---------------------- sbin/hastd/metadata.c | 16 ++++++++++------ sbin/hastd/nv.c | 2 ++ sbin/hastd/primary.c | 6 +++++- sbin/hastd/secondary.c | 3 ++- 7 files changed, 41 insertions(+), 40 deletions(-) diff --git a/sbin/hastd/ebuf.c b/sbin/hastd/ebuf.c index 47b7530e..0bccd181 100644 --- a/sbin/hastd/ebuf.c +++ b/sbin/hastd/ebuf.c @@ -55,8 +55,8 @@ struct ebuf { size_t eb_size; }; -static int ebuf_head_extent(struct ebuf *eb, size_t size); -static int ebuf_tail_extent(struct ebuf *eb, size_t size); +static int ebuf_head_extend(struct ebuf *eb, size_t size); +static int ebuf_tail_extend(struct ebuf *eb, size_t size); struct ebuf * ebuf_alloc(size_t size) @@ -110,7 +110,7 @@ ebuf_add_head(struct ebuf *eb, const void *data, size_t size) * We can't add more entries at the front, so we have to extend * our buffer. */ - if (ebuf_head_extent(eb, size) < 0) + if (ebuf_head_extend(eb, size) < 0) return (-1); } assert(size <= (size_t)(eb->eb_used - eb->eb_start)); @@ -137,13 +137,13 @@ ebuf_add_tail(struct ebuf *eb, const void *data, size_t size) * We can't add more entries at the back, so we have to extend * our buffer. */ - if (ebuf_tail_extent(eb, size) < 0) + if (ebuf_tail_extend(eb, size) < 0) return (-1); } assert(size <= (size_t)(eb->eb_end - (eb->eb_used + eb->eb_size))); /* - * If data is NULL the caller just wants to reserve place. + * If data is NULL the caller just wants to reserve space. */ if (data != NULL) bcopy(data, eb->eb_used + eb->eb_size, size); @@ -203,7 +203,7 @@ ebuf_size(struct ebuf *eb) * Function adds size + (PAGE_SIZE / 4) bytes at the front of the buffer.. */ static int -ebuf_head_extent(struct ebuf *eb, size_t size) +ebuf_head_extend(struct ebuf *eb, size_t size) { unsigned char *newstart, *newused; size_t newsize; @@ -231,7 +231,7 @@ ebuf_head_extent(struct ebuf *eb, size_t size) * Function adds size + ((3 * PAGE_SIZE) / 4) bytes at the back. */ static int -ebuf_tail_extent(struct ebuf *eb, size_t size) +ebuf_tail_extend(struct ebuf *eb, size_t size) { unsigned char *newstart; size_t newsize; diff --git a/sbin/hastd/hast_proto.c b/sbin/hastd/hast_proto.c index 348dfc88..4e898ca1 100644 --- a/sbin/hastd/hast_proto.c +++ b/sbin/hastd/hast_proto.c @@ -329,9 +329,7 @@ hast_proto_recv_hdr(struct proto_conn *conn, struct nv **nvp) *nvp = nv; return (0); fail: - if (nv != NULL) - nv_free(nv); - else if (eb != NULL) + if (eb != NULL) ebuf_free(eb); return (-1); } diff --git a/sbin/hastd/hastd.c b/sbin/hastd/hastd.c index 27b9bba4..1d9d1b4e 100644 --- a/sbin/hastd/hastd.c +++ b/sbin/hastd/hastd.c @@ -200,7 +200,7 @@ listen_accept(void) proto_local_address(conn, laddr, sizeof(laddr)); proto_remote_address(conn, raddr, sizeof(raddr)); - pjdlog_info("Connection from %s to %s.", laddr, raddr); + pjdlog_info("Connection from %s to %s.", raddr, laddr); /* Error in setting timeout is not critical, but why should it fail? */ if (proto_timeout(conn, HAST_TIMEOUT) < 0) @@ -286,7 +286,7 @@ listen_accept(void) if (token != NULL && memcmp(token, res->hr_token, sizeof(res->hr_token)) != 0) { pjdlog_error("Token received from %s doesn't match.", raddr); - nv_add_stringf(nverr, "errmsg", "Toke doesn't match."); + nv_add_stringf(nverr, "errmsg", "Token doesn't match."); goto fail; } /* @@ -400,7 +400,11 @@ static void main_loop(void) { fd_set rfds, wfds; - int fd, maxfd, ret; + int cfd, lfd, maxfd, ret; + + cfd = proto_descriptor(cfg->hc_controlconn); + lfd = proto_descriptor(cfg->hc_listenconn); + maxfd = cfd > lfd ? cfd : lfd; for (;;) { if (sigchld_received) { @@ -412,22 +416,13 @@ main_loop(void) hastd_reload(); } - maxfd = 0; + /* Setup descriptors for select(2). */ FD_ZERO(&rfds); + FD_SET(cfd, &rfds); + FD_SET(lfd, &rfds); FD_ZERO(&wfds); - - /* Setup descriptors for select(2). */ -#define SETUP_FD(conn) do { \ - fd = proto_descriptor(conn); \ - if (fd >= 0) { \ - maxfd = fd > maxfd ? fd : maxfd; \ - FD_SET(fd, &rfds); \ - FD_SET(fd, &wfds); \ - } \ -} while (0) - SETUP_FD(cfg->hc_controlconn); - SETUP_FD(cfg->hc_listenconn); -#undef SETUP_FD + FD_SET(cfd, &wfds); + FD_SET(lfd, &wfds); ret = select(maxfd + 1, &rfds, &wfds, NULL, NULL); if (ret == -1) { @@ -437,13 +432,10 @@ main_loop(void) pjdlog_exit(EX_OSERR, "select() failed"); } -#define ISSET_FD(conn) \ - (FD_ISSET((fd = proto_descriptor(conn)), &rfds) || FD_ISSET(fd, &wfds)) - if (ISSET_FD(cfg->hc_controlconn)) + if (FD_ISSET(cfd, &rfds) || FD_ISSET(cfd, &wfds)) control_handle(cfg); - if (ISSET_FD(cfg->hc_listenconn)) + if (FD_ISSET(lfd, &rfds) || FD_ISSET(lfd, &wfds)) listen_accept(); -#undef ISSET_FD } } diff --git a/sbin/hastd/metadata.c b/sbin/hastd/metadata.c index 7a138e81..de21b53e 100644 --- a/sbin/hastd/metadata.c +++ b/sbin/hastd/metadata.c @@ -96,6 +96,7 @@ metadata_read(struct hast_resource *res, bool openrw) rerrno = errno; pjdlog_errno(LOG_ERR, "Unable to allocate memory to read metadata"); + ebuf_free(eb); goto fail; } buf = ebuf_data(eb, NULL); @@ -154,6 +155,7 @@ metadata_read(struct hast_resource *res, bool openrw) nv_free(nv); goto fail; } + nv_free(nv); return (0); fail: if (opened_here) { @@ -172,6 +174,7 @@ metadata_write(struct hast_resource *res) unsigned char *buf, *ptr; size_t size; ssize_t done; + int ret; buf = calloc(1, METADATA_SIZE); if (buf == NULL) { @@ -180,6 +183,8 @@ metadata_write(struct hast_resource *res) return (-1); } + ret = -1; + nv = nv_alloc(); nv_add_string(nv, res->hr_name, "resource"); nv_add_uint64(nv, (uint64_t)res->hr_datasize, "datasize"); @@ -199,7 +204,7 @@ metadata_write(struct hast_resource *res) nv_add_string(nv, role2str(res->hr_role), "prevrole"); if (nv_error(nv) != 0) { pjdlog_error("Unable to create metadata."); - goto fail; + goto end; } res->hr_previous_role = res->hr_role; eb = nv_hton(nv); @@ -211,12 +216,11 @@ metadata_write(struct hast_resource *res) done = pwrite(res->hr_localfd, buf, METADATA_SIZE, 0); if (done < 0 || done != METADATA_SIZE) { pjdlog_errno(LOG_ERR, "Unable to write metadata"); - goto fail; + goto end; } - - return (0); -fail: + ret = 0; +end: free(buf); nv_free(nv); - return (-1); + return (ret); } diff --git a/sbin/hastd/nv.c b/sbin/hastd/nv.c index 0b4e362c..49b0cbbc 100644 --- a/sbin/hastd/nv.c +++ b/sbin/hastd/nv.c @@ -707,8 +707,10 @@ nv_add(struct nv *nv, const unsigned char *value, size_t vsize, int type, assert(errno != 0); if (nv->nv_error == 0) nv->nv_error = errno; + free(nvh); return; } + free(nvh); /* Add the actual data. */ if (ebuf_add_tail(nv->nv_ebuf, value, vsize) < 0) { assert(errno != 0); diff --git a/sbin/hastd/primary.c b/sbin/hastd/primary.c index 9f2b2c79..ae742204 100644 --- a/sbin/hastd/primary.c +++ b/sbin/hastd/primary.c @@ -195,7 +195,10 @@ static pthread_mutex_t metadata_lock; mtx_unlock(&hio_##name##_list_lock); \ } while (0) -#define SYNCREQ(hio) do { (hio)->hio_ggio.gctl_unit = -1; } while (0) +#define SYNCREQ(hio) do { \ + (hio)->hio_ggio.gctl_unit = -1; \ + (hio)->hio_ggio.gctl_seq = 1; \ +} while (0) #define ISSYNCREQ(hio) ((hio)->hio_ggio.gctl_unit == -1) #define SYNCREQDONE(hio) do { (hio)->hio_ggio.gctl_unit = -2; } while (0) #define ISSYNCREQDONE(hio) ((hio)->hio_ggio.gctl_unit == -2) @@ -447,6 +450,7 @@ init_local(struct hast_resource *res) primary_exit(EX_NOINPUT, "Unable to read activemap"); } activemap_copyin(res->hr_amp, buf, mapsize); + free(buf); if (res->hr_resuid != 0) return; /* diff --git a/sbin/hastd/secondary.c b/sbin/hastd/secondary.c index b189b7e2..b487e9dc 100644 --- a/sbin/hastd/secondary.c +++ b/sbin/hastd/secondary.c @@ -295,6 +295,7 @@ init_remote(struct hast_resource *res, struct nv *nvin) nv_free(nvout); exit(EX_TEMPFAIL); } + nv_free(nvout); if (res->hr_secondary_localcnt > res->hr_primary_remotecnt && res->hr_primary_localcnt > res->hr_secondary_remotecnt) { /* Exit on split-brain. */ @@ -687,7 +688,7 @@ send_thread(void *arg) pjdlog_exit(EX_TEMPFAIL, "Unable to send reply."); } nv_free(nvout); - pjdlog_debug(2, "disk: (%p) Moving request to the free queue.", + pjdlog_debug(2, "send: (%p) Moving request to the free queue.", hio); nv_free(hio->hio_nv); hio->hio_error = 0; -- 2.42.0