Remove cli->event_ctx, pass it explicitly

Storing the event_context as permanent state in struct cli_state creates more
complex code than necessary IMO.
(This used to be commit debb37f703)

source3/include/async_smb.h

@@ -23,7 +23,9 @@
  * Ship a new smb request to the server
  */
 
-struct async_req *cli_request_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_request_send(TALLOC_CTX *mem_ctx,
+				   struct event_context *ev,
+				   struct cli_state *cli,
 				   uint8_t smb_command,
 				   uint8_t additional_flags,
 				   uint8_t wct, const uint16_t *vwv,
@@ -46,17 +48,3 @@ NTSTATUS cli_pull_error(char *buf);
  */
 
 void cli_set_error(struct cli_state *cli, NTSTATUS status);
-
-/*
- * Create a temporary event context for use in the sync helper functions
- */
-
-struct cli_tmp_event *cli_tmp_event_ctx(TALLOC_CTX *mem_ctx,
-					struct cli_state *cli);
-
-/*
- * Attach an event context permanently to a cli_struct
- */
-
-NTSTATUS cli_add_event_ctx(struct cli_state *cli,
-			   struct event_context *event_ctx);

source3/include/client.h

@@ -204,7 +204,15 @@ struct cli_state {
 	bool force_dos_errors;
 	bool case_sensitive; /* False by default. */
 
-	struct event_context *event_ctx;
+	/**
+	 * fd_event is around while we have async requests outstanding or are
+	 * building a chained request.
+	 *
+	 * (fd_event!=NULL) && (outstanding_request!=NULL)
+	 *
+	 * should always be true, as well as the reverse: If both cli_request
+	 * pointers are NULL, no fd_event is around.
+	 */
 	struct fd_event *fd_event;
 	char *evt_inbuf;
 

source3/include/proto.h

@@ -4378,7 +4378,8 @@ int cli_nt_create_full(struct cli_state *cli, const char *fname,
 		 uint32 CreateDisposition, uint32 CreateOptions,
 		 uint8 SecuityFlags);
 int cli_nt_create(struct cli_state *cli, const char *fname, uint32 DesiredAccess);
-struct async_req *cli_open_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_open_send(TALLOC_CTX *mem_ctx, struct event_context *ev,
+				struct cli_state *cli,
 				const char *fname, int flags, int share_mode);
 NTSTATUS cli_open_recv(struct async_req *req, int *fnum);
 int cli_open(struct cli_state *cli, const char *fname, int flags, int share_mode);
@@ -4618,11 +4619,14 @@ int cli_NetConnectionEnum(struct cli_state *cli, const char *qualifier,
 /* The following definitions come from libsmb/clireadwrite.c  */
 
 struct async_req *cli_read_andx_send(TALLOC_CTX *mem_ctx,
+				     struct event_context *ev,
 				     struct cli_state *cli, int fnum,
 				     off_t offset, size_t size);
 NTSTATUS cli_read_andx_recv(struct async_req *req, ssize_t *received,
 			    uint8_t **rcvbuf);
-struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx,
+				struct event_context *ev,
+				struct cli_state *cli,
 				uint16_t fnum, off_t start_offset,
 				SMB_OFF_T size, size_t window_size,
 				NTSTATUS (*sink)(char *buf, size_t n,

source3/libsmb/async_smb.c

@@ -19,6 +19,9 @@
 
 #include "includes.h"
 
+static void cli_state_handler(struct event_context *event_ctx,
+			      struct fd_event *event, uint16 flags, void *p);
+
 /*
  * Fetch an error out of a NBT packet
  */
@@ -107,6 +110,9 @@ static int cli_request_destructor(struct cli_request *req)
 		common_free_enc_buffer(req->enc_state, req->outbuf);
 	}
 	DLIST_REMOVE(req->cli->outstanding_requests, req);
+	if (req->cli->outstanding_requests == NULL) {
+		TALLOC_FREE(req->cli->fd_event);
+	}
 	return 0;
 }
 
@@ -163,7 +169,9 @@ static struct async_req *cli_request_new(TALLOC_CTX *mem_ctx,
 /*
  * Ship a new smb request to the server
  */
-struct async_req *cli_request_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_request_send(TALLOC_CTX *mem_ctx,
+				   struct event_context *ev,
+				   struct cli_state *cli,
 				   uint8_t smb_command,
 				   uint8_t additional_flags,
 				   uint8_t wct, const uint16_t *vwv,
@@ -172,10 +180,20 @@ struct async_req *cli_request_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
 	struct async_req *result;
 	struct cli_request *req;
 
-	result = cli_request_new(mem_ctx, cli->event_ctx, cli, wct, num_bytes,
+	if (cli->fd_event == NULL) {
-				 &req);
+		SMB_ASSERT(cli->outstanding_requests == NULL);
+		cli->fd_event = event_add_fd(ev, cli, cli->fd,
+					     EVENT_FD_READ,
+					     cli_state_handler, cli);
+		if (cli->fd_event == NULL) {
+			return NULL;
+		}
+	}
+
+	result = cli_request_new(mem_ctx, ev, cli, wct, num_bytes, &req);
 	if (result == NULL) {
 		DEBUG(0, ("cli_request_new failed\n"));
+		TALLOC_FREE(cli->fd_event);
 		return NULL;
 	}
 
@@ -446,7 +464,9 @@ static void cli_state_handler(struct event_context *event_ctx,
 		}
 
 		if (req == NULL) {
-			event_fd_set_not_writeable(event);
+			if (cli->fd_event != NULL) {
+				event_fd_set_not_writeable(cli->fd_event);
+			}
 			return;
 		}
 
@@ -474,69 +494,3 @@ static void cli_state_handler(struct event_context *event_ctx,
 	close(cli->fd);
 	cli->fd = -1;
 }
-
-/*
- * Holder for a talloc_destructor, we need to zero out the pointers in cli
- * when deleting
- */
-struct cli_tmp_event {
-	struct cli_state *cli;
-};
-
-static int cli_tmp_event_destructor(struct cli_tmp_event *e)
-{
-	TALLOC_FREE(e->cli->fd_event);
-	TALLOC_FREE(e->cli->event_ctx);
-	return 0;
-}
-
-/*
- * Create a temporary event context for use in the sync helper functions
- */
-
-struct cli_tmp_event *cli_tmp_event_ctx(TALLOC_CTX *mem_ctx,
-					struct cli_state *cli)
-{
-	struct cli_tmp_event *state;
-
-	if (cli->event_ctx != NULL) {
-		return NULL;
-	}
-
-	state = talloc(mem_ctx, struct cli_tmp_event);
-	if (state == NULL) {
-		return NULL;
-	}
-	state->cli = cli;
-	talloc_set_destructor(state, cli_tmp_event_destructor);
-
-	cli->event_ctx = event_context_init(state);
-	if (cli->event_ctx == NULL) {
-		TALLOC_FREE(state);
-		return NULL;
-	}
-
-	cli->fd_event = event_add_fd(cli->event_ctx, state, cli->fd,
-				     EVENT_FD_READ, cli_state_handler, cli);
-	if (cli->fd_event == NULL) {
-		TALLOC_FREE(state);
-		return NULL;
-	}
-	return state;
-}
-
-/*
- * Attach an event context permanently to a cli_struct
- */
-
-NTSTATUS cli_add_event_ctx(struct cli_state *cli,
-			   struct event_context *event_ctx)
-{
-	cli->event_ctx = event_ctx;
-	cli->fd_event = event_add_fd(event_ctx, cli, cli->fd, EVENT_FD_READ,
-				     cli_state_handler, cli);
-	if (cli->fd_event == NULL) {
-		return NT_STATUS_NO_MEMORY;
-	}
-	return NT_STATUS_OK;
-}

source3/libsmb/clifile.c

@@ -823,7 +823,8 @@ static uint8_t *smb_bytes_push_str(uint8_t *buf, bool ucs2, const char *str)
  WARNING: if you open with O_WRONLY then getattrE won't work!
 ****************************************************************************/
 
-struct async_req *cli_open_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_open_send(TALLOC_CTX *mem_ctx, struct event_context *ev,
+				struct cli_state *cli,
 				const char *fname, int flags, int share_mode)
 {
 	unsigned openfn = 0;
@@ -893,7 +894,7 @@ struct async_req *cli_open_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
 		return NULL;
 	}
 
-	result = cli_request_send(mem_ctx, cli, SMBopenX, additional_flags,
+	result = cli_request_send(mem_ctx, ev, cli, SMBopenX, additional_flags,
 				  15, vwv, talloc_get_size(bytes), bytes);
 	TALLOC_FREE(bytes);
 	return result;
@@ -923,20 +924,30 @@ int cli_open(struct cli_state *cli, const char *fname, int flags,
 	     int share_mode)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
+	struct event_context *ev;
 	struct async_req *req;
 	int result = -1;
 
-	if (cli_tmp_event_ctx(frame, cli) == NULL) {
+	if (cli->fd_event != NULL) {
+		/*
+		 * Can't use sync call while an async call is in flight
+		 */
+		cli_set_error(cli, NT_STATUS_INVALID_PARAMETER);
 		goto fail;
 	}
 
-	req = cli_open_send(frame, cli, fname, flags, share_mode);
+	ev = event_context_init(frame);
+	if (ev == NULL) {
+		goto fail;
+	}
+
+	req = cli_open_send(frame, ev, cli, fname, flags, share_mode);
 	if (req == NULL) {
 		goto fail;
 	}
 
 	while (req->state < ASYNC_REQ_DONE) {
-		event_loop_once(cli->event_ctx);
+		event_loop_once(ev);
 	}
 
 	cli_open_recv(req, &result);
@@ -949,15 +960,16 @@ int cli_open(struct cli_state *cli, const char *fname, int flags,
  Close a file.
 ****************************************************************************/
 
-struct async_req *cli_close_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_close_send(TALLOC_CTX *mem_ctx, struct event_context *ev,
-				 int fnum)
+				 struct cli_state *cli, int fnum)
 {
 	uint16_t vwv[3];
 
 	SSVAL(vwv+0, 0, fnum);
 	SIVALS(vwv+1, 0, -1);
 
-	return cli_request_send(mem_ctx, cli, SMBclose, 0, 3, vwv, 0, NULL);
+	return cli_request_send(mem_ctx, ev, cli, SMBclose, 0, 3, vwv,
+				0, NULL);
 }
 
 NTSTATUS cli_close_recv(struct async_req *req)
@@ -975,20 +987,30 @@ NTSTATUS cli_close_recv(struct async_req *req)
 bool cli_close(struct cli_state *cli, int fnum)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
+	struct event_context *ev;
 	struct async_req *req;
 	bool result = false;
 
-	if (cli_tmp_event_ctx(frame, cli) == NULL) {
+	if (cli->fd_event != NULL) {
+		/*
+		 * Can't use sync call while an async call is in flight
+		 */
+		cli_set_error(cli, NT_STATUS_INVALID_PARAMETER);
 		goto fail;
 	}
 
-	req = cli_close_send(frame, cli, fnum);
+	ev = event_context_init(frame);
+	if (ev == NULL) {
+		goto fail;
+	}
+
+	req = cli_close_send(frame, ev, cli, fnum);
 	if (req == NULL) {
 		goto fail;
 	}
 
 	while (req->state < ASYNC_REQ_DONE) {
-		event_loop_once(cli->event_ctx);
+		event_loop_once(ev);
 	}
 
 	result = NT_STATUS_IS_OK(cli_close_recv(req));

source3/libsmb/clireadwrite.c

@@ -41,6 +41,7 @@ static size_t cli_read_max_bufsize(struct cli_state *cli)
  */
 
 struct async_req *cli_read_andx_send(TALLOC_CTX *mem_ctx,
+				     struct event_context *ev,
 				     struct cli_state *cli, int fnum,
 				     off_t offset, size_t size)
 {
@@ -76,7 +77,7 @@ struct async_req *cli_read_andx_send(TALLOC_CTX *mem_ctx,
 		wct += 2;
 	}
 
-	result = cli_request_send(mem_ctx, cli, SMBreadX, 0, wct, vwv,
+	result = cli_request_send(mem_ctx, ev, cli, SMBreadX, 0, wct, vwv,
 				  0, NULL);
 	if (result == NULL) {
 		return NULL;
@@ -144,6 +145,7 @@ NTSTATUS cli_read_andx_recv(struct async_req *req, ssize_t *received,
 struct cli_pull_state {
 	struct async_req *req;
 
+	struct event_context *ev;
 	struct cli_state *cli;
 	uint16_t fnum;
 	off_t start_offset;
@@ -202,7 +204,9 @@ static void cli_pull_read_done(struct async_req *read_req);
  * Prepare an async pull request
  */
 
-struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
+struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx,
+				struct event_context *ev,
+				struct cli_state *cli,
 				uint16_t fnum, off_t start_offset,
 				SMB_OFF_T size, size_t window_size,
 				NTSTATUS (*sink)(char *buf, size_t n,
@@ -213,7 +217,7 @@ struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
 	struct cli_pull_state *state;
 	int i;
 
-	result = async_req_new(mem_ctx, cli->event_ctx);
+	result = async_req_new(mem_ctx, ev);
 	if (result == NULL) {
 		goto failed;
 	}
@@ -226,6 +230,7 @@ struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
 	state->req = result;
 
 	state->cli = cli;
+	state->ev = ev;
 	state->fnum = fnum;
 	state->start_offset = start_offset;
 	state->size = size;
@@ -268,7 +273,7 @@ struct async_req *cli_pull_send(TALLOC_CTX *mem_ctx, struct cli_state *cli,
 		request_thistime = MIN(size_left, state->chunk_size);
 
 		state->reqs[i] = cli_read_andx_send(
-			state->reqs, cli, fnum,
+			state->reqs, ev, cli, fnum,
 			state->start_offset + state->requested,
 			request_thistime);
 
@@ -363,7 +368,8 @@ static void cli_pull_read_done(struct async_req *read_req)
 				   state->top_req));
 
 			new_req = cli_read_andx_send(
-				state->reqs, state->cli, state->fnum,
+				state->reqs, state->ev, state->cli,
+				state->fnum,
 				state->start_offset + state->requested,
 				request_thistime);
 
@@ -403,21 +409,30 @@ NTSTATUS cli_pull(struct cli_state *cli, uint16_t fnum,
 		  void *priv, SMB_OFF_T *received)
 {
 	TALLOC_CTX *frame = talloc_stackframe();
+	struct event_context *ev;
 	struct async_req *req;
 	NTSTATUS result = NT_STATUS_NO_MEMORY;
 
-	if (cli_tmp_event_ctx(frame, cli) == NULL) {
+	if (cli->fd_event != NULL) {
+		/*
+		 * Can't use sync call while an async call is in flight
+		 */
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
+	ev = event_context_init(frame);
+	if (ev == NULL) {
 		goto nomem;
 	}
 
-	req = cli_pull_send(frame, cli, fnum, start_offset, size, window_size,
+	req = cli_pull_send(frame, ev, cli, fnum, start_offset, size,
-			    sink, priv);
+			    window_size, sink, priv);
 	if (req == NULL) {
 		goto nomem;
 	}
 
 	while (req->state < ASYNC_REQ_DONE) {
-		event_loop_once(cli->event_ctx);
+		event_loop_once(ev);
 	}
 
 	result = cli_pull_recv(req, received);