The branch, master has been updated
via 7bc2e2d messaging_dgm: Don't expose the messaging_dgm_context
via 9269a8e messaging_dgm: Avoid a talloc
via 55a894c messaging_dgm: Avoid a talloc
via c95d9a5 messaging_dgm: messaging_dgm_lockfile_remove does not use
tmp_ctx anymore
via 9c5b1ee messaging_dgm: messaging_dgm_lockfile_create does not use
tmp_ctx anymore
via c3f98b9 messaging_dgm: Avoid talloc_tos()
via 2858c66 messaging_dgm: Reformatting
via 188120b messaging_dgm: Remove an unnecessary cast
from 0cd4561 namecache: fix uninitialised pointer returns
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 7bc2e2d0d4a6706769a436798aa29d18a8f7c221
Author: Volker Lendecke <v...@samba.org>
Date: Wed Sep 10 16:13:18 2014 +0200
messaging_dgm: Don't expose the messaging_dgm_context
Right now we can only support one messaging_dgm_context per process
anyway, the code has checks for this. I would like to keep it that
way, in the future we will have multiple messaging_context's or
imessaging_context's filtering based upon the dst server_id.
Why this change? messaging_dgm's lockfile contains the
serverid->unique_id. When designing messaging_dgm, I had in mind to
remove the serverid.tdb and replace it with the dgm lockfiles for server
lookup and enumeration. I have a WIP-patchset that gets rid of almost
all users of serverid.tdb. The problem is serverid_exists. Here we don't
have a messaging_context available, and it would be pretty intrusive
to make it so. This problem has plagued us since ctdb was developed,
see for example the comment
/*
* This is a Samba3 hack/optimization. Routines like process_exists need to
* talk to ctdbd, and they don't get handed a messaging context.
*/
in messaging_ctdb.c. This patchset removes this problem in a radical way:
Treat the messaging_dgm context as one globally available structure and
be done with it. The ctdb socket could go the same way in the future.
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
Autobuild-User(master): Stefan Metzmacher <me...@samba.org>
Autobuild-Date(master): Sun Sep 14 16:29:30 CEST 2014 on sn-devel-104
commit 9269a8e5d4f92569cf529e530bf800971c23682d
Author: Volker Lendecke <v...@samba.org>
Date: Wed Sep 10 10:12:23 2014 +0200
messaging_dgm: Avoid a talloc
Not really required, but it removes a NULL check
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 55a894c71d97a1210c3261d6272e5e5a6078b909
Author: Volker Lendecke <v...@samba.org>
Date: Wed Sep 10 10:12:23 2014 +0200
messaging_dgm: Avoid a talloc
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit c95d9a55d136da72d70aea78c7bd50e932c0105e
Author: Volker Lendecke <v...@samba.org>
Date: Wed Sep 10 10:00:05 2014 +0200
messaging_dgm: messaging_dgm_lockfile_remove does not use tmp_ctx anymore
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 9c5b1eeaf68c3d3c4d138dcb0008a018030aa39a
Author: Volker Lendecke <v...@samba.org>
Date: Wed Sep 10 10:00:05 2014 +0200
messaging_dgm: messaging_dgm_lockfile_create does not use tmp_ctx anymore
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit c3f98b978965353bdf2dbad7d80b3f62cdb5672e
Author: Volker Lendecke <v...@samba.org>
Date: Wed Sep 10 09:58:00 2014 +0200
messaging_dgm: Avoid talloc_tos()
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 2858c666b51710f526269bd8495996a026bba7f3
Author: Volker Lendecke <v...@samba.org>
Date: Tue Sep 9 21:46:19 2014 +0200
messaging_dgm: Reformatting
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
commit 188120b08cac87ae53bae07bbf90ebde777d84e8
Author: Volker Lendecke <v...@samba.org>
Date: Tue Sep 9 21:45:58 2014 +0200
messaging_dgm: Remove an unnecessary cast
Signed-off-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
source3/lib/messages.c | 33 ++++----
source3/lib/messages_dgm.c | 195 ++++++++++++++++++++++++--------------------
source3/lib/messages_dgm.h | 16 ++--
3 files changed, 131 insertions(+), 113 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 80ecec4..52d6538 100644
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -73,8 +73,6 @@ struct messaging_context {
struct tevent_req **waiters;
unsigned num_waiters;
- struct messaging_dgm_context *local;
-
struct messaging_backend *remote;
};
@@ -240,6 +238,12 @@ static void messaging_recv_cb(const uint8_t *msg, size_t
msg_len,
messaging_dispatch_rec(msg_ctx, &rec);
}
+static int messaging_context_destructor(struct messaging_context *ctx)
+{
+ messaging_dgm_destroy();
+ return 0;
+}
+
struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev)
{
@@ -256,9 +260,9 @@ struct messaging_context *messaging_init(TALLOC_CTX
*mem_ctx,
sec_init();
- ret = messaging_dgm_init(ctx, ctx->event_ctx, ctx->id,
+ ret = messaging_dgm_init(ctx->event_ctx, ctx->id,
lp_cache_directory(), sec_initial_uid(),
- messaging_recv_cb, ctx, &ctx->local);
+ messaging_recv_cb, ctx);
if (ret != 0) {
DEBUG(2, ("messaging_dgm_init failed: %s\n", strerror(ret)));
@@ -266,6 +270,8 @@ struct messaging_context *messaging_init(TALLOC_CTX
*mem_ctx,
return NULL;
}
+ talloc_set_destructor(ctx, messaging_context_destructor);
+
if (lp_clustering()) {
status = messaging_ctdbd_init(ctx, ctx, &ctx->remote);
@@ -302,14 +308,13 @@ NTSTATUS messaging_reinit(struct messaging_context
*msg_ctx)
NTSTATUS status;
int ret;
- TALLOC_FREE(msg_ctx->local);
+ messaging_dgm_destroy();
msg_ctx->id = procid_self();
- ret = messaging_dgm_init(msg_ctx, msg_ctx->event_ctx, msg_ctx->id,
+ ret = messaging_dgm_init(msg_ctx->event_ctx, msg_ctx->id,
lp_cache_directory(), sec_initial_uid(),
- messaging_recv_cb, msg_ctx,
- &msg_ctx->local);
+ messaging_recv_cb, msg_ctx);
if (ret != 0) {
DEBUG(0, ("messaging_dgm_init failed: %s\n", strerror(errno)));
return map_nt_error_from_unix(ret);
@@ -480,7 +485,7 @@ NTSTATUS messaging_send_iov(struct messaging_context
*msg_ctx,
memcpy(&iov2[1], iov, iovlen * sizeof(*iov));
become_root();
- ret = messaging_dgm_send(msg_ctx->local, server.pid, iov2, iovlen+1);
+ ret = messaging_dgm_send(server.pid, iov2, iovlen+1);
unbecome_root();
if (ret != 0) {
@@ -549,7 +554,7 @@ struct tevent_req *messaging_filtered_read_send(
tevent_req_defer_callback(req, state->ev);
state->tevent_handle = messaging_dgm_register_tevent_context(
- state, msg_ctx->local, ev);
+ state, ev);
if (tevent_req_nomem(state, req)) {
return tevent_req_post(req, ev);
}
@@ -908,11 +913,9 @@ bool messaging_parent_dgm_cleanup_init(struct
messaging_context *msg)
static int mess_parent_dgm_cleanup(void *private_data)
{
- struct messaging_context *msg_ctx = talloc_get_type_abort(
- private_data, struct messaging_context);
int ret;
- ret = messaging_dgm_wipe(msg_ctx->local);
+ ret = messaging_dgm_wipe();
DEBUG(10, ("messaging_dgm_wipe returned %s\n",
ret ? strerror(ret) : "ok"));
return lp_parm_int(-1, "messaging", "messaging dgm cleanup interval",
@@ -946,9 +949,9 @@ int messaging_cleanup(struct messaging_context *msg_ctx,
pid_t pid)
int ret;
if (pid == 0) {
- ret = messaging_dgm_wipe(msg_ctx->local);
+ ret = messaging_dgm_wipe();
} else {
- ret = messaging_dgm_cleanup(msg_ctx->local, pid);
+ ret = messaging_dgm_cleanup(pid);
}
return ret;
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 659b835..471aed4 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -27,12 +27,19 @@
#include "poll_funcs/poll_funcs_tevent.h"
#include "unix_msg/unix_msg.h"
+struct sun_path_buf {
+ /*
+ * This will carry enough for a socket path
+ */
+ char buf[sizeof(struct sockaddr_un)];
+};
+
struct messaging_dgm_context {
pid_t pid;
struct poll_funcs *msg_callbacks;
void *tevent_handle;
struct unix_msg_ctx *dgm_ctx;
- char *cache_dir;
+ struct sun_path_buf cache_dir;
int lockfile_fd;
void (*recv_cb)(const uint8_t *msg,
@@ -43,41 +50,50 @@ struct messaging_dgm_context {
bool *have_dgm_context;
};
+static struct messaging_dgm_context *global_dgm_context;
+
static void messaging_dgm_recv(struct unix_msg_ctx *ctx,
uint8_t *msg, size_t msg_len,
void *private_data);
-static char *messaging_dgm_lockfile_name(TALLOC_CTX *mem_ctx,
- const char *cache_dir,
- pid_t pid)
+static int messaging_dgm_lockfile_name(struct sun_path_buf *buf,
+ const char *cache_dir,
+ pid_t pid)
{
- return talloc_asprintf(mem_ctx, "%s/lck/%u", cache_dir,
- (unsigned)pid);
+ int ret;
+
+ ret = snprintf(buf->buf, sizeof(buf->buf), "%s/lck/%u", cache_dir,
+ (unsigned)pid);
+ if (ret >= sizeof(buf->buf)) {
+ return ENAMETOOLONG;
+ }
+ return 0;
}
static int messaging_dgm_context_destructor(struct messaging_dgm_context *c);
-static int messaging_dgm_lockfile_create(TALLOC_CTX *tmp_ctx,
- const char *cache_dir,
+static int messaging_dgm_lockfile_create(const char *cache_dir,
uid_t dir_owner, pid_t pid,
int *plockfile_fd, uint64_t unique)
{
fstring buf;
- char *dir;
- char *lockfile_name;
+ struct sun_path_buf dir;
+ struct sun_path_buf lockfile_name;
int lockfile_fd;
struct flock lck;
int unique_len, ret;
ssize_t written;
bool ok;
- dir = talloc_asprintf(tmp_ctx, "%s/lck", cache_dir);
- if (dir == NULL) {
- return ENOMEM;
+ ret = messaging_dgm_lockfile_name(&lockfile_name, cache_dir, pid);
+ if (ret != 0) {
+ return ret;
}
- ok = directory_create_or_exist_strict(dir, dir_owner, 0755);
- TALLOC_FREE(dir);
+ /* shorter than lockfile_name, can't overflow */
+ snprintf(dir.buf, sizeof(dir.buf), "%s/lck", cache_dir);
+
+ ok = directory_create_or_exist_strict(dir.buf, dir_owner, 0755);
if (!ok) {
ret = errno;
DEBUG(1, ("%s: Could not create lock directory: %s\n",
@@ -85,19 +101,14 @@ static int messaging_dgm_lockfile_create(TALLOC_CTX
*tmp_ctx,
return ret;
}
- lockfile_name = messaging_dgm_lockfile_name(tmp_ctx, cache_dir,
- (unsigned)pid);
- if (lockfile_name == NULL) {
- return ENOMEM;
- }
-
/* no O_EXCL, existence check is via the fcntl lock */
- lockfile_fd = open(lockfile_name, O_NONBLOCK|O_CREAT|O_WRONLY, 0644);
+ lockfile_fd = open(lockfile_name.buf, O_NONBLOCK|O_CREAT|O_WRONLY,
+ 0644);
if (lockfile_fd == -1) {
ret = errno;
DEBUG(1, ("%s: open failed: %s\n", __func__, strerror(errno)));
- goto fail_free;
+ return ret;
}
lck = (struct flock) {
@@ -131,57 +142,49 @@ static int messaging_dgm_lockfile_create(TALLOC_CTX
*tmp_ctx,
goto fail_unlink;
}
- TALLOC_FREE(lockfile_name);
*plockfile_fd = lockfile_fd;
return 0;
fail_unlink:
- unlink(lockfile_name);
+ unlink(lockfile_name.buf);
fail_close:
close(lockfile_fd);
-fail_free:
- TALLOC_FREE(lockfile_name);
return ret;
}
-static int messaging_dgm_lockfile_remove(TALLOC_CTX *tmp_ctx,
- const char *cache_dir, pid_t pid)
+static int messaging_dgm_lockfile_remove(const char *cache_dir, pid_t pid)
{
- char *lockfile_name;
+ struct sun_path_buf lockfile_name;
int ret;
- lockfile_name = messaging_dgm_lockfile_name(
- tmp_ctx, cache_dir, pid);
- if (lockfile_name == NULL) {
- return ENOMEM;
+ ret = messaging_dgm_lockfile_name(&lockfile_name, cache_dir, pid);
+ if (ret != 0) {
+ return ret;
}
- ret = unlink(lockfile_name);
+ ret = unlink(lockfile_name.buf);
if (ret == -1) {
ret = errno;
DEBUG(10, ("%s: unlink(%s) failed: %s\n", __func__,
- lockfile_name, strerror(ret)));
+ lockfile_name.buf, strerror(ret)));
}
- TALLOC_FREE(lockfile_name);
return ret;
}
-int messaging_dgm_init(TALLOC_CTX *mem_ctx,
- struct tevent_context *ev,
+int messaging_dgm_init(struct tevent_context *ev,
struct server_id pid,
const char *cache_dir,
uid_t dir_owner,
void (*recv_cb)(const uint8_t *msg,
size_t msg_len,
void *private_data),
- void *recv_cb_private_data,
- struct messaging_dgm_context **pctx)
+ void *recv_cb_private_data)
{
struct messaging_dgm_context *ctx;
int ret;
bool ok;
- char *socket_dir;
+ struct sun_path_buf socket_dir;
struct sockaddr_un socket_address;
size_t sockname_len;
uint64_t cookie;
@@ -191,7 +194,7 @@ int messaging_dgm_init(TALLOC_CTX *mem_ctx,
return EEXIST;
}
- ctx = talloc_zero(mem_ctx, struct messaging_dgm_context);
+ ctx = talloc_zero(NULL, struct messaging_dgm_context);
if (ctx == NULL) {
goto fail_nomem;
}
@@ -199,25 +202,26 @@ int messaging_dgm_init(TALLOC_CTX *mem_ctx,
ctx->recv_cb = recv_cb;
ctx->recv_cb_private_data = recv_cb_private_data;
- ctx->cache_dir = talloc_strdup(ctx, cache_dir);
- if (ctx->cache_dir == NULL) {
- goto fail_nomem;
- }
- socket_dir = talloc_asprintf(ctx, "%s/msg", cache_dir);
- if (socket_dir == NULL) {
- goto fail_nomem;
+ ret = snprintf(socket_dir.buf, sizeof(socket_dir.buf),
+ "%s/msg", cache_dir);
+ if (ret >= sizeof(socket_dir.buf)) {
+ TALLOC_FREE(ctx);
+ return ENAMETOOLONG;
}
+ /* shorter than socket_dir, can't overflow */
+ strlcpy(ctx->cache_dir.buf, cache_dir, sizeof(ctx->cache_dir.buf));
+
socket_address = (struct sockaddr_un) { .sun_family = AF_UNIX };
sockname_len = snprintf(socket_address.sun_path,
sizeof(socket_address.sun_path),
- "%s/%u", socket_dir, (unsigned)pid.pid);
+ "%s/%u", socket_dir.buf, (unsigned)pid.pid);
if (sockname_len >= sizeof(socket_address.sun_path)) {
TALLOC_FREE(ctx);
return ENAMETOOLONG;
}
- ret = messaging_dgm_lockfile_create(ctx, cache_dir, dir_owner, pid.pid,
+ ret = messaging_dgm_lockfile_create(cache_dir, dir_owner, pid.pid,
&ctx->lockfile_fd, pid.unique_id);
if (ret != 0) {
DEBUG(1, ("%s: messaging_dgm_create_lockfile failed: %s\n",
@@ -237,13 +241,12 @@ int messaging_dgm_init(TALLOC_CTX *mem_ctx,
goto fail_nomem;
}
- ok = directory_create_or_exist_strict(socket_dir, dir_owner, 0700);
+ ok = directory_create_or_exist_strict(socket_dir.buf, dir_owner, 0700);
if (!ok) {
DEBUG(1, ("Could not create socket directory\n"));
TALLOC_FREE(ctx);
return EACCES;
}
- TALLOC_FREE(socket_dir);
unlink(socket_address.sun_path);
@@ -260,7 +263,7 @@ int messaging_dgm_init(TALLOC_CTX *mem_ctx,
ctx->have_dgm_context = &have_dgm_context;
- *pctx = ctx;
+ global_dgm_context = ctx;
return 0;
fail_nomem:
@@ -277,7 +280,7 @@ static int messaging_dgm_context_destructor(struct
messaging_dgm_context *c)
unix_msg_free(c->dgm_ctx);
if (getpid() == c->pid) {
- (void)messaging_dgm_lockfile_remove(c, c->cache_dir, c->pid);
+ (void)messaging_dgm_lockfile_remove(c->cache_dir.buf, c->pid);
}
close(c->lockfile_fd);
@@ -288,17 +291,26 @@ static int messaging_dgm_context_destructor(struct
messaging_dgm_context *c)
return 0;
}
-int messaging_dgm_send(struct messaging_dgm_context *ctx, pid_t pid,
- const struct iovec *iov, int iovlen)
+void messaging_dgm_destroy(void)
+{
+ TALLOC_FREE(global_dgm_context);
+}
+
+int messaging_dgm_send(pid_t pid, const struct iovec *iov, int iovlen)
{
+ struct messaging_dgm_context *ctx = global_dgm_context;
struct sockaddr_un dst;
ssize_t dst_pathlen;
int ret;
+ if (ctx == NULL) {
+ return ENOTCONN;
+ }
+
dst = (struct sockaddr_un) { .sun_family = AF_UNIX };
dst_pathlen = snprintf(dst.sun_path, sizeof(dst.sun_path),
- "%s/msg/%u", ctx->cache_dir, (unsigned)pid);
+ "%s/msg/%u", ctx->cache_dir.buf, (unsigned)pid);
if (dst_pathlen >= sizeof(dst.sun_path)) {
return ENAMETOOLONG;
}
@@ -320,32 +332,34 @@ static void messaging_dgm_recv(struct unix_msg_ctx *ctx,
dgm_ctx->recv_cb(msg, msg_len, dgm_ctx->recv_cb_private_data);
}
-int messaging_dgm_cleanup(struct messaging_dgm_context *ctx, pid_t pid)
+int messaging_dgm_cleanup(pid_t pid)
{
- char *lockfile_name, *socket_name;
+ struct messaging_dgm_context *ctx = global_dgm_context;
+ struct sun_path_buf lockfile_name, socket_name;
int fd, ret;
struct flock lck = {};
- lockfile_name = messaging_dgm_lockfile_name(
- talloc_tos(), ctx->cache_dir, pid);
- if (lockfile_name == NULL) {
- return ENOMEM;
+ if (ctx == NULL) {
+ return ENOTCONN;
}
- socket_name = talloc_asprintf(lockfile_name, "%s/msg/%u",
- ctx->cache_dir, (unsigned)pid);
- if (socket_name == NULL) {
- TALLOC_FREE(lockfile_name);
- return ENOMEM;
+
+ ret = messaging_dgm_lockfile_name(&lockfile_name, ctx->cache_dir.buf,
+ pid);
+ if (ret != 0) {
+ return ret;
}
- fd = open(lockfile_name, O_NONBLOCK|O_WRONLY, 0);
+ /* same length as lockfile_name, can't overflow */
+ snprintf(socket_name.buf, sizeof(socket_name.buf), "%s/msg/%u",
+ ctx->cache_dir.buf, (unsigned)pid);
+
+ fd = open(lockfile_name.buf, O_NONBLOCK|O_WRONLY, 0);
if (fd == -1) {
ret = errno;
if (ret != ENOENT) {
DEBUG(10, ("%s: open(%s) failed: %s\n", __func__,
- lockfile_name, strerror(ret)));
+ lockfile_name.buf, strerror(ret)));
}
- TALLOC_FREE(lockfile_name);
return ret;
}
@@ -359,45 +373,46 @@ int messaging_dgm_cleanup(struct messaging_dgm_context
*ctx, pid_t pid)
ret = errno;
DEBUG(10, ("%s: Could not get lock: %s\n", __func__,
strerror(ret)));
- TALLOC_FREE(lockfile_name);
close(fd);
return ret;
}
- (void)unlink(socket_name);
- (void)unlink(lockfile_name);
+ (void)unlink(socket_name.buf);
+ (void)unlink(lockfile_name.buf);
(void)close(fd);
-
- TALLOC_FREE(lockfile_name);
return 0;
}
-int messaging_dgm_wipe(struct messaging_dgm_context *ctx)
+int messaging_dgm_wipe(void)
{
- char *msgdir_name;
+ struct messaging_dgm_context *ctx = global_dgm_context;
+ struct sun_path_buf msgdir_name;
DIR *msgdir;
struct dirent *dp;
pid_t our_pid = getpid();
int ret;
+ if (ctx == NULL) {
+ return ENOTCONN;
+ }
+
/*
* We scan the socket directory and not the lock directory. Otherwise
* we would race against messaging_dgm_lockfile_create's open(O_CREAT)
* and fcntl(SETLK).
*/
- msgdir_name = talloc_asprintf(talloc_tos(), "%s/msg", ctx->cache_dir);
- if (msgdir_name == NULL) {
- return ENOMEM;
+ ret = snprintf(msgdir_name.buf, sizeof(msgdir_name.buf),
+ "%s/msg", ctx->cache_dir.buf);
+ if (ret >= sizeof(msgdir_name.buf)) {
+ return ENAMETOOLONG;
}
- msgdir = opendir(msgdir_name);
+ msgdir = opendir(msgdir_name.buf);
if (msgdir == NULL) {
ret = errno;
--
Samba Shared Repository
