[sheepdog] [PATCH v2] sheep: add a helper function to copy out data from strbuf

MORITA Kazutaka morita.kazutaka at lab.ntt.co.jp
Tue May 29 17:35:55 CEST 2012


This patch also fixes a problem that local_get_store_list() doesn't
set a null terminated string.

Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
---

Sorry for the late reply.  This is the updated version.

Changes from v1:
 - add the third argument to avoid buffer overflow
 - add documentation


 doc/api-strbuf.txt   |    6 ++++++
 include/strbuf.h     |    1 +
 lib/strbuf.c         |    8 ++++++++
 sheep/farm/farm.c    |    4 ++--
 sheep/object_cache.c |    2 +-
 sheep/ops.c          |    2 +-
 sheep/recovery.c     |    5 +++--
 7 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/doc/api-strbuf.txt b/doc/api-strbuf.txt
index 151c7c3..c10caef 100644
--- a/doc/api-strbuf.txt
+++ b/doc/api-strbuf.txt
@@ -197,3 +197,9 @@ same behaviour as well.
 	Reading stops after the terminator or at EOF.  The terminator
 	is removed from the buffer before returning.  Returns 0 unless
 	there was nothing left before EOF, in which case it returns `EOF`.
+
+`strbuf_copyout`::
+
+	Copy the contents of the strbuf to the second argument 'buf'.
+	The number of bytes to be copied is at most the third argument
+	'len'.
diff --git a/include/strbuf.h b/include/strbuf.h
index 68e644b..7ffa903 100644
--- a/include/strbuf.h
+++ b/include/strbuf.h
@@ -96,5 +96,6 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
 /* XXX: if read fails, any partial read is undone */
 extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
 int strbuf_getline(struct strbuf *sb, FILE *fp, int term);
+extern int strbuf_copyout(struct strbuf *sb, void *buf, size_t len);
 
 #endif
diff --git a/lib/strbuf.c b/lib/strbuf.c
index f9377c3..d0c3c27 100644
--- a/lib/strbuf.c
+++ b/lib/strbuf.c
@@ -187,3 +187,11 @@ int strbuf_getline(struct strbuf *sb, FILE *fp, int term)
 		strbuf_setlen(sb, sb->len-1);
 	return 0;
 }
+
+int strbuf_copyout(struct strbuf *sb, void *buf, size_t len)
+{
+	len = min(len, sb->len + 1);
+	memcpy(buf, sb->buf, len);
+
+	return len;
+}
diff --git a/sheep/farm/farm.c b/sheep/farm/farm.c
index b50d188..cbfe99d 100644
--- a/sheep/farm/farm.c
+++ b/sheep/farm/farm.c
@@ -41,7 +41,7 @@ static int create_directory(char *p)
 	}
 
 	if (!strlen(farm_dir))
-		memcpy(farm_dir, buf.buf, buf.len);
+		strbuf_copyout(&buf, farm_dir, sizeof(farm_dir));
 
 	strbuf_addstr(&buf, "/objects");
 	if (mkdir(buf.buf, 0755) < 0) {
@@ -64,7 +64,7 @@ static int create_directory(char *p)
 	}
 
 	if (!strlen(farm_obj_dir))
-		memcpy(farm_obj_dir, buf.buf, buf.len);
+		strbuf_copyout(&buf, farm_obj_dir, sizeof(farm_obj_dir));
 err:
 	strbuf_release(&buf);
 	return ret;
diff --git a/sheep/object_cache.c b/sheep/object_cache.c
index 7d2733d..85e1d9f 100644
--- a/sheep/object_cache.c
+++ b/sheep/object_cache.c
@@ -753,7 +753,7 @@ int object_cache_init(const char *p)
 			goto err;
 		}
 	}
-	memcpy(cache_dir, buf.buf, buf.len);
+	strbuf_copyout(&buf, cache_dir, sizeof(cache_dir));
 err:
 	strbuf_release(&buf);
 	return ret;
diff --git a/sheep/ops.c b/sheep/ops.c
index ad74733..27d7743 100644
--- a/sheep/ops.c
+++ b/sheep/ops.c
@@ -314,7 +314,7 @@ static int local_get_store_list(struct request *req)
 	list_for_each_entry(driver, &store_drivers, list) {
 		strbuf_addf(&buf, "%s ", driver->name);
 	}
-	memcpy(req->data, buf.buf, buf.len);
+	strbuf_copyout(&buf, req->data, req->data_length);
 
 	strbuf_release(&buf);
 	return SD_RES_SUCCESS;
diff --git a/sheep/recovery.c b/sheep/recovery.c
index 9b71d9b..9960978 100644
--- a/sheep/recovery.c
+++ b/sheep/recovery.c
@@ -544,6 +544,7 @@ static int screen_obj_list(struct recovery_work *rw,  uint64_t *list, int list_n
 	struct strbuf buf = STRBUF_INIT;
 	struct sd_vnode *vnodes[SD_MAX_COPIES];
 	int nr_objs;
+	size_t len;
 
 	nr_objs = get_nr_copies(rw->cur_vnodes);
 	for (i = 0; i < list_nr; i++) {
@@ -555,9 +556,9 @@ static int screen_obj_list(struct recovery_work *rw,  uint64_t *list, int list_n
 			}
 		}
 	}
-	memcpy(list, buf.buf, buf.len);
+	len = strbuf_copyout(&buf, list, list_nr * sizeof(uint64_t));
 
-	ret = buf.len / sizeof(uint64_t);
+	ret = len / sizeof(uint64_t);
 	dprintf("%d\n", ret);
 	strbuf_release(&buf);
 
-- 
1.7.2.5




More information about the sheepdog mailing list