aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Seebach <peter.seebach@windriver.com>2014-04-22 17:04:23 -0500
committerPeter Seebach <peter.seebach@windriver.com>2014-04-22 17:04:23 -0500
commit5392d265efd29e374080dca062ff1d1a834bfbed (patch)
treefedf5a7db8974343ae3848ada4d5da7956f8873f
parent1c0d6d9a3cbb193a1c2f0e4f57a70781e7b69931 (diff)
downloadpseudo-5392d265efd29e374080dca062ff1d1a834bfbed.tar.gz
pseudo-5392d265efd29e374080dca062ff1d1a834bfbed.tar.bz2
pseudo-5392d265efd29e374080dca062ff1d1a834bfbed.zip
xattr support and other path stuff: reduce allocation and copying
The xattr first-pass implementation was allocating a buffer to hold the name and value for a set operation, then pseudo_client was allocating *another* buffer to hold the path and those two values. pseudo_client_op develops more nuanced argument handling, and also uses a static buffer for the extended paths it sometimes needs. So for the typical use case, only occasional operations will need to reallocate/expand the buffer, and we'll be down to copying things into that buffer once per operation, instead of having two alloc/free pairs and two copies. And of course, that wasn't two alloc/free pairs, it was one alloc/free pair and one alloc without a free. Whoops. Signed-off-by: Peter Seebach <peter.seebach@windriver.com>
-rw-r--r--ports/linux/xattr/pseudo_wrappers.c54
-rw-r--r--pseudo.h9
-rw-r--r--pseudo_client.c114
-rw-r--r--pseudo_db.c2
-rw-r--r--pseudo_util.c37
5 files changed, 141 insertions, 75 deletions
diff --git a/ports/linux/xattr/pseudo_wrappers.c b/ports/linux/xattr/pseudo_wrappers.c
index c37c245..40699d6 100644
--- a/ports/linux/xattr/pseudo_wrappers.c
+++ b/ports/linux/xattr/pseudo_wrappers.c
@@ -112,7 +112,7 @@ posix_permissions(const acl_header *header, int entries, int *extra, int *mode)
static ssize_t shared_getxattr(const char *path, int fd, const char *name, void *value, size_t size) {
RC_AND_BUF
- pseudo_debug(PDBGF_XATTR, "getxattr(%s/%d, %s)\n",
+ pseudo_debug(PDBGF_XATTR, "getxattr(%s [fd %d], %s)\n",
path ? path : "<no path>", fd, name);
pseudo_msg_t *result = pseudo_client_op(OP_GET_XATTR, 0, fd, -1, path, &buf, name);
if (result->result != RESULT_SUCCEED) {
@@ -135,31 +135,10 @@ static ssize_t shared_getxattr(const char *path, int fd, const char *name, void
static int shared_setxattr(const char *path, int fd, const char *name, const void *value, size_t size, int flags) {
RC_AND_BUF
-
- char *combined;
- size_t nlen = strlen(name);
- size_t combined_len = nlen + size + 1;
- combined = malloc(combined_len + 1);
- memcpy(combined, name, nlen);
- combined[nlen] = '\0';
- memcpy(combined + nlen + 1, value, size);
- combined[combined_len] = '\0';
-
- pseudo_debug(PDBGF_XATTR, "setxattr(%s/%d, %s, %s => %s [%d])\n",
- path ? path : "<no path>", fd, name, (char *) value, combined + nlen + 1, (int) size);
-
pseudo_op_t op;
- switch (flags) {
- case XATTR_CREATE:
- op = OP_CREATE_XATTR;
- break;
- case XATTR_REPLACE:
- op = OP_REPLACE_XATTR;
- break;
- default:
- op = OP_SET_XATTR;
- break;
- }
+
+ pseudo_debug(PDBGF_XATTR, "setxattr(%s [fd %d], %s => '%.*s')\n",
+ path ? path : "<no path>", fd, name, (int) size, (char *) value);
/* this may be a plain chmod */
if (!strcmp(name, "system.posix_acl_access")) {
@@ -170,7 +149,16 @@ static int shared_setxattr(const char *path, int fd, const char *name, const voi
pseudo_debug(PDBGF_XATTR, "posix_acl_access translated to mode %04o. Remaining attribute(s): %d.\n",
mode, extra);
buf.st_mode = mode;
- pseudo_client_op(path ? OP_CHMOD : OP_FCHMOD, 0, fd, -1, path, &buf);
+ /* we want to actually issue a corresponding chmod,
+ * as well, or else the file ends up 0600 on the
+ * host. Using the slightly-less-efficient wrap_chmod
+ * avoids possible misalignment.
+ */
+ if (path) {
+ wrap_chmod(path, mode);
+ } else {
+ wrap_fchmod(fd, mode);
+ }
/* we are sneaky, and do not actually record this using
* extended attributes. */
if (!extra) {
@@ -179,7 +167,19 @@ static int shared_setxattr(const char *path, int fd, const char *name, const voi
}
}
- pseudo_msg_t *result = pseudo_client_op(op, 0, fd, -1, path, &buf, combined, combined_len);
+ switch (flags) {
+ case XATTR_CREATE:
+ op = OP_CREATE_XATTR;
+ break;
+ case XATTR_REPLACE:
+ op = OP_REPLACE_XATTR;
+ break;
+ default:
+ op = OP_SET_XATTR;
+ break;
+ }
+
+ pseudo_msg_t *result = pseudo_client_op(op, 0, fd, -1, path, &buf, name, value, size);
/* we automatically assume success */
if (op == OP_SET_XATTR) {
diff --git a/pseudo.h b/pseudo.h
index ad78686..62678b7 100644
--- a/pseudo.h
+++ b/pseudo.h
@@ -49,13 +49,22 @@ extern int pseudo_diag(char *, ...) __attribute__ ((format (printf, 1, 2)));
if (pseudo_util_debug_flags & (x)) { pseudo_diag(__VA_ARGS__); } \
} \
} while (0)
+#define pseudo_debug_call(x, fn, ...) do { \
+ if ((x) & PDBGF_VERBOSE) { \
+ if ((pseudo_util_debug_flags & PDBGF_VERBOSE) && (pseudo_util_debug_flags & ((x) & ~PDBGF_VERBOSE))) { fn(__VA_ARGS__); } \
+ } else { \
+ if (pseudo_util_debug_flags & (x)) { fn(__VA_ARGS__); } \
+ } \
+} while (0)
#else
/* this used to be a static inline function, but that meant that arguments
* were still evaluated for side effects. We don't want that. The ...
* is a C99ism, also supported by GNU C.
*/
#define pseudo_debug(...) 0
+#define pseudo_debug_call(...) 0
#endif
+extern void pseudo_dump_data(char *, const void *, size_t);
void pseudo_new_pid(void);
/* pseudo_fix_path resolves symlinks up to this depth */
#define PSEUDO_MAX_LINK_RECURSION 16
diff --git a/pseudo_client.c b/pseudo_client.c
index 8c39e6b..7330f9d 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -1055,9 +1055,12 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
pseudo_msg_t msg = { .type = PSEUDO_MSG_OP };
size_t pathlen = -1;
int do_request = 0;
- char *oldpath = 0;
- size_t oldpathlen = 0;
- char *alloced_path = 0;
+ char *path_extra_1 = 0;
+ size_t path_extra_1len = 0;
+ char *path_extra_2 = 0;
+ size_t path_extra_2len = 0;
+ static char *alloced_path = 0;
+ static size_t alloced_len = 0;
int strip_slash;
/* disable wrappers */
@@ -1065,27 +1068,25 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
if (op == OP_RENAME) {
va_list ap;
- va_start(ap, buf);
- oldpath = va_arg(ap, char *);
- va_end(ap);
- /* last argument is the previous path of the file */
- if (!oldpath) {
- pseudo_diag("rename (%s) without old path.\n",
+ if (!path) {
+ pseudo_diag("rename (%s) without new path.\n",
path ? path : "<nil>");
pseudo_magic();
return 0;
}
- /* we have to calculate this here, because SET_XATTR
- * and friends will be using oldpath to hold a hunk of
- * data of arbitrary length
- */
- oldpathlen = strlen(oldpath);
- if (!path) {
- pseudo_diag("rename (%s) without new path.\n",
+ va_start(ap, buf);
+ path_extra_1 = va_arg(ap, char *);
+ va_end(ap);
+ /* last argument is the previous path of the file */
+ if (!path_extra_1) {
+ pseudo_diag("rename (%s) without old path.\n",
path ? path : "<nil>");
pseudo_magic();
return 0;
}
+ path_extra_1len = strlen(path_extra_1);
+ pseudo_debug(PDBGF_PATH | PDBGF_FILE, "rename: %s -> %s\n",
+ path_extra_1, path);
}
/* we treat the "create" and "replace" flags as logically
@@ -1094,20 +1095,23 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
if (op == OP_SET_XATTR || op == OP_CREATE_XATTR || op == OP_REPLACE_XATTR) {
va_list ap;
va_start(ap, buf);
- oldpath = va_arg(ap, char *);
- oldpathlen = va_arg(ap, size_t);
- pseudo_debug(PDBGF_XATTR, "setxattr, oldpath (%d bytes): '%s'\n",
- (int) oldpathlen, oldpath);
+ path_extra_1 = va_arg(ap, char *);
+ path_extra_1len = strlen(path_extra_1);
+ path_extra_2 = va_arg(ap, char *);
+ path_extra_2len = va_arg(ap, size_t);
va_end(ap);
+ pseudo_debug(PDBGF_XATTR, "setxattr, name '%s', value %d bytes\n",
+ path_extra_1, (int) path_extra_2len);
+ pseudo_debug_call(PDBGF_XATTR | PDBGF_VERBOSE, pseudo_dump_data, "xattr value", path_extra_2, path_extra_2len);
}
- if (op == OP_GET_XATTR){
+ if (op == OP_GET_XATTR || op == OP_REMOVE_XATTR) {
va_list ap;
va_start(ap, buf);
- oldpath = va_arg(ap, char *);
- oldpathlen = strlen(oldpath);
- pseudo_debug(PDBGF_XATTR, "getxattr, oldpath (%d bytes): '%s'\n",
- (int) oldpathlen, oldpath);
+ path_extra_1 = va_arg(ap, char *);
+ path_extra_1len = strlen(path_extra_1);
va_end(ap);
+ pseudo_debug(PDBGF_XATTR, "%sxattr, name '%s'\n",
+ op == OP_GET_XATTR ? "get" : "remove", path_extra_1);
}
/* if path isn't available, try to find one? */
@@ -1143,27 +1147,48 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
* value even though we don't have one available. We use an
* empty path for that.
*/
- if (oldpath) {
- size_t full_len = oldpathlen + 1 + pathlen;
+ if (path_extra_1) {
+ size_t full_len = path_extra_1len + 1 + pathlen;
size_t partial_len = pathlen - 1 - strip_slash;
- char *both_paths = malloc(full_len);
- if (!both_paths) {
- pseudo_diag("Can't allocate space for paths for a rename operation. Sorry.\n");
- pseudo_magic();
- return 0;
+ if (path_extra_2) {
+ full_len += path_extra_2len + 1;
+ }
+ if (full_len > alloced_len) {
+ free(alloced_path);
+ alloced_path = malloc(full_len);
+ alloced_len = full_len;
+ if (!alloced_path) {
+ pseudo_diag("Can't allocate space for paths for a rename operation. Sorry.\n");
+ alloced_len = 0;
+ pseudo_magic();
+ return 0;
+ }
}
- memcpy(both_paths, path, partial_len);
- both_paths[partial_len] = '\0';
- memcpy(both_paths + partial_len + 1, oldpath, oldpathlen);
- both_paths[full_len - 1] = '\0';
- pseudo_debug(PDBGF_PATH | PDBGF_FILE, "rename: %s -> %s [%d]\n",
- both_paths + pathlen, both_paths, (int) full_len);
- alloced_path = both_paths;
+ memcpy(alloced_path, path, partial_len);
+ alloced_path[partial_len] = '\0';
+ memcpy(alloced_path + partial_len + 1, path_extra_1, path_extra_1len);
+ alloced_path[partial_len + path_extra_1len + 1] = '\0';
+ if (path_extra_2) {
+ memcpy(alloced_path + partial_len + path_extra_1len + 2, path_extra_2, path_extra_2len);
+ }
+ alloced_path[full_len - 1] = '\0';
path = alloced_path;
pathlen = full_len;
+ pseudo_debug_call(PDBGF_IPC | PDBGF_VERBOSE, pseudo_dump_data, "combined path buffer", path, pathlen);
} else {
if (strip_slash) {
- alloced_path = strdup(path);
+ if (pathlen > alloced_len) {
+ free(alloced_path);
+ alloced_path = malloc(pathlen);
+ alloced_len = pathlen;
+ if (!alloced_path) {
+ pseudo_diag("Can't allocate space for paths for a rename operation. Sorry.\n");
+ alloced_len = 0;
+ pseudo_magic();
+ return 0;
+ }
+ }
+ memcpy(alloced_path, path, pathlen);
alloced_path[pathlen - 2] = '\0';
path = alloced_path;
}
@@ -1171,8 +1196,8 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
pseudo_debug(PDBGF_OP, "%s%s", pseudo_op_name(op),
(dirfd != -1 && dirfd != AT_FDCWD && op != OP_DUP) ? "at" : "");
- if (oldpath) {
- pseudo_debug(PDBGF_OP, " %s ->", (char *) oldpath);
+ if (path_extra_1) {
+ pseudo_debug(PDBGF_OP, " %s ->", path_extra_1);
}
if (path) {
pseudo_debug(PDBGF_OP, " %s", path);
@@ -1339,11 +1364,6 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
}
pseudo_debug(PDBGF_OP, "\n");
- /* if not NULL, alloced_path is an allocated buffer for both
- * paths, or for modified paths...
- */
- free(alloced_path);
-
if (do_request && (messages % 1000 == 0)) {
pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE | PDBGF_BENCHMARK, "%d messages handled in %.4f seconds\n",
messages,
diff --git a/pseudo_db.c b/pseudo_db.c
index 0a20b96..c7b6585 100644
--- a/pseudo_db.c
+++ b/pseudo_db.c
@@ -2376,7 +2376,7 @@ pdb_set_xattr(long long file_id, char *value, size_t len, int flags) {
/* the material after the name is the value */
vlen = strlen(value);
len = len - (vlen + 1);
- value = value + len;
+ value = value + vlen + 1;
pseudo_debug(PDBGF_XATTR, "trying to set a value for %lld: name is '%s' [%d/%d bytes], value is '%s'. Existing row %lld.\n",
file_id, vname, (int) vlen, (int) (len + vlen + 1), value, existing_row);
if (existing_row != -1) {
diff --git a/pseudo_util.c b/pseudo_util.c
index a4a14b4..f20dbec 100644
--- a/pseudo_util.c
+++ b/pseudo_util.c
@@ -1432,3 +1432,40 @@ pseudo_stat64_from32(struct stat64 *buf64, const struct stat *buf) {
buf64->st_mtime = buf->st_mtime;
buf64->st_ctime = buf->st_ctime;
}
+
+/* pretty-dump some data.
+ * expects to be called using pseudo_debug_call() so it doesn't have
+ * to do debug checks.
+ */
+void
+pseudo_dump_data(char *name, const void *v, size_t len) {
+ char hexbuf[128];
+ char asciibuf[32];
+ const unsigned char *base = v;
+ const unsigned char *data = base;
+ int remaining = len;
+ pseudo_diag("%s at %p [%d byte%s]:\n",
+ name ? name : "data", v, (int) len, len == 1 ? "" : "s");
+ while (remaining > 0) {
+ char *hexptr = hexbuf;
+ char *asciiptr = asciibuf;
+ for (int i = 0; i < 16 && i < remaining; ++i) {
+ hexptr += snprintf(hexptr, 4, "%02x ", data[i]);
+ if (isprint(data[i])) {
+ *asciiptr++ = data[i];
+ } else {
+ *asciiptr++ = '.';
+ }
+ if (i % 4 == 3) {
+ *hexptr++ = ' ';
+ }
+ }
+ *hexptr = '\0';
+ *asciiptr = '\0';
+ pseudo_diag("0x%06x %-50.50s '%.16s'\n",
+ (int) (data - base),
+ hexbuf, asciibuf);
+ remaining = remaining - 16;
+ data = data + 16;
+ }
+}