libsmb: Convert cli_list_recv() to single-recv

This converts the higher-level cli_list_recv() to the new
cli_smb2_list_recv() calling convention to just issue one entry per
recv() call in preparation of using the async cli_smb2_list_send() in
cli_list_send().

For SMB1 this will be a performance degradation, as we have to make
copies out of the arrays that cli_trans_recv() returns, but soon this
will become a performance improvement for the SMB2 directory
listing. And as hopefully most deployments these days are SMB2, I
think we can live with the SMB1 client directory listing
degradation. Also, we can also convert the lowerlevel SMB1 directory
listing routines in case someone actually sees problems from this
here.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c
index d0abd476d749b3831a2ca2441e5dd6c369fe85b1..d4841207f29c2e123f2ac75004d98b05b5c12e3f 100644 (file)
--- a/source3/libsmb/clilist.c
+++ b/source3/libsmb/clilist.c
@@ -970,9 +970,11 @@ NTSTATUS cli_list_trans(struct cli_state *cli, const char *mask,
 }
 
 struct cli_list_state {
+       struct tevent_context *ev;
        NTSTATUS (*recv_fn)(struct tevent_req *req, TALLOC_CTX *mem_ctx,
                            struct file_info **finfo);
        struct file_info *finfo;
+       size_t num_received;
 };
 
 static void cli_list_done(struct tevent_req *subreq);
@@ -991,6 +993,7 @@ struct tevent_req *cli_list_send(TALLOC_CTX *mem_ctx,
        if (req == NULL) {
                return NULL;
        }
+       state->ev = ev;
 
        if (smbXcli_conn_protocol(cli->conn) <= PROTOCOL_LANMAN1) {
                subreq = cli_list_old_send(state, ev, cli, mask, attribute);
@@ -1021,22 +1024,110 @@ static void cli_list_done(struct tevent_req *subreq)
                tevent_req_nterror(req, status);
                return;
        }
-       tevent_req_done(req);
+       tevent_req_defer_callback(req, state->ev);
+       tevent_req_notify_callback(req);
 }
 
-NTSTATUS cli_list_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
-                      struct file_info **finfo, size_t *num_finfo)
+NTSTATUS cli_list_recv(
+       struct tevent_req *req,
+       TALLOC_CTX *mem_ctx,
+       struct file_info **pfinfo)
 {
        struct cli_list_state *state = tevent_req_data(
                req, struct cli_list_state);
+       size_t num_results;
+       struct file_info *finfo = NULL;
        NTSTATUS status;
+       bool in_progress;
 
-       if (tevent_req_is_nterror(req, &status)) {
-               return status;
+       in_progress = tevent_req_is_in_progress(req);
+
+       if (!in_progress) {
+               if (!tevent_req_is_nterror(req, &status)) {
+                       status = NT_STATUS_NO_MORE_FILES;
+               }
+               goto fail;
        }
-       *num_finfo = talloc_array_length(state->finfo);
-       *finfo = talloc_move(mem_ctx, &state->finfo);
+
+       num_results = talloc_array_length(state->finfo);
+       if (state->num_received == num_results) {
+               status = NT_STATUS_NO_MORE_FILES;
+               goto fail;
+       }
+
+       if (num_results == 1) {
+               finfo = talloc_move(mem_ctx, &state->finfo);
+       } else {
+               struct file_info *src_finfo =
+                       &state->finfo[state->num_received];
+
+               finfo = talloc(mem_ctx, struct file_info);
+               if (finfo == NULL) {
+                       goto nomem;
+               }
+               *finfo = *src_finfo;
+               finfo->name = talloc_move(finfo, &src_finfo->name);
+               finfo->short_name = talloc_move(finfo, &src_finfo->short_name);
+       }
+
+       state->num_received += 1;
+
+       tevent_req_defer_callback(req, state->ev);
+       tevent_req_notify_callback(req);
+
+       *pfinfo = finfo;
        return NT_STATUS_OK;
+
+nomem:
+       status = NT_STATUS_NO_MEMORY;
+fail:
+       TALLOC_FREE(finfo);
+       tevent_req_received(req);
+       return status;
+}
+
+struct cli_list_sync_state {
+       const char *mask;
+       uint32_t attribute;
+       NTSTATUS (*fn)(struct file_info *finfo,
+                      const char *mask,
+                      void *private_data);
+       void *private_data;
+       NTSTATUS status;
+       bool processed_file;
+};
+
+static void cli_list_sync_cb(struct tevent_req *subreq)
+{
+       struct cli_list_sync_state *state =
+               tevent_req_callback_data_void(subreq);
+       struct file_info *finfo;
+       bool ok;
+
+       state->status = cli_list_recv(subreq, talloc_tos(), &finfo);
+       /* No TALLOC_FREE(subreq), we get here more than once */
+
+       if (!NT_STATUS_IS_OK(state->status)) {
+               return;
+       }
+
+       ok = dir_check_ftype(finfo->attr, state->attribute);
+       if (!ok) {
+               /*
+                * Only process if attributes match.  On SMB1 server
+                * does this, so on SMB2 we need to emulate in the
+                * client.
+                *
+                * https://bugzilla.samba.org/show_bug.cgi?id=10260
+                */
+               return;
+       }
+
+       state->status = state->fn(finfo, state->mask, state->private_data);
+
+       state->processed_file = true;
+
+       TALLOC_FREE(finfo);
 }
 
 NTSTATUS cli_list(struct cli_state *cli,
@@ -1048,11 +1139,15 @@ NTSTATUS cli_list(struct cli_state *cli,
                  void *private_data)
 {
        TALLOC_CTX *frame = NULL;
+       struct cli_list_sync_state state = {
+               .mask = mask,
+               .attribute = attribute,
+               .fn = fn,
+               .private_data = private_data,
+       };
        struct tevent_context *ev;
        struct tevent_req *req;
        NTSTATUS status = NT_STATUS_NO_MEMORY;
-       struct file_info *finfo;
-       size_t i, num_finfo = 0;
        uint16_t info_level;
 
        if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
@@ -1080,21 +1175,22 @@ NTSTATUS cli_list(struct cli_state *cli,
        if (req == NULL) {
                goto fail;
        }
+       tevent_req_set_callback(req, cli_list_sync_cb, &state);
+
        if (!tevent_req_poll_ntstatus(req, ev, &status)) {
                goto fail;
        }
 
-       status = cli_list_recv(req, frame, &finfo, &num_finfo);
-       if (!NT_STATUS_IS_OK(status)) {
-               goto fail;
+       status = state.status;
+
+       if (NT_STATUS_EQUAL(status, NT_STATUS_NO_MORE_FILES)) {
+               status = NT_STATUS_OK;
        }
 
-       for (i=0; i<num_finfo; i++) {
-               status = fn(&finfo[i], mask, private_data);
-               if (!NT_STATUS_IS_OK(status)) {
-                       goto fail;
-               }
+       if (NT_STATUS_IS_OK(status) && !state.processed_file) {
+               status = NT_STATUS_NO_SUCH_FILE;
        }
+
  fail:
        TALLOC_FREE(frame);
        return status;

diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index a4c807de2f8e7028fac314fc985616b9de42a515..64ba498e3696ed73f68cafb0bca0896065934438 100644 (file)
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -757,8 +757,10 @@ struct tevent_req *cli_list_send(TALLOC_CTX *mem_ctx,
                                 const char *mask,
                                 uint32_t attribute,
                                 uint16_t info_level);
-NTSTATUS cli_list_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
-                      struct file_info **finfo, size_t *num_finfo);
+NTSTATUS cli_list_recv(
+       struct tevent_req *req,
+       TALLOC_CTX *mem_ctx,
+       struct file_info **pfinfo);
 NTSTATUS cli_list(struct cli_state *cli,
                  const char *mask,
                  uint32_t attribute,

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 5d106df9d0567bc23269f393a8108e983f442510..a1d48fe863e62b77b483332169b18f2b0e200550 100644 (file)
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -1146,6 +1146,29 @@ static NTSTATUS list_helper(struct file_info *finfo,
        return NT_STATUS_OK;
 }
 
+struct do_listing_state {
+       const char *mask;
+       NTSTATUS (*callback_fn)(
+               struct file_info *finfo,
+               const char *mask,
+               void *private_data);
+       void *private_data;
+       NTSTATUS status;
+};
+
+static void do_listing_cb(struct tevent_req *subreq)
+{
+       struct do_listing_state *state = tevent_req_callback_data_void(subreq);
+       struct file_info *finfo = NULL;
+
+       state->status = cli_list_recv(subreq, NULL, &finfo);
+       if (!NT_STATUS_IS_OK(state->status)) {
+               return;
+       }
+       state->callback_fn(finfo, state->mask, state->private_data);
+       TALLOC_FREE(finfo);
+}
+
 static NTSTATUS do_listing(struct py_cli_state *self,
                           const char *base_dir, const char *user_mask,
                           uint16_t attribute,
@@ -1155,9 +1178,6 @@ static NTSTATUS do_listing(struct py_cli_state *self,
 {
        char *mask = NULL;
        unsigned int info_level = SMB_FIND_FILE_BOTH_DIRECTORY_INFO;
-       struct file_info *finfos = NULL;
-       size_t i;
-       size_t num_finfos = 0;
        NTSTATUS status;
 
        if (user_mask == NULL) {
@@ -1172,36 +1192,37 @@ static NTSTATUS do_listing(struct py_cli_state *self,
        dos_format(mask);
 
        if (self->is_smb1) {
+               struct do_listing_state state = {
+                       .mask = mask,
+                       .callback_fn = callback_fn,
+                       .private_data = priv,
+               };
                struct tevent_req *req = NULL;
 
                req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
                                    info_level);
+               if (req == NULL) {
+                       status = NT_STATUS_NO_MEMORY;
+                       goto done;
+               }
+               tevent_req_set_callback(req, do_listing_cb, &state);
+
                if (!py_tevent_req_wait_exc(self, req)) {
                        return NT_STATUS_INTERNAL_ERROR;
                }
-               status = cli_list_recv(req, NULL, &finfos, &num_finfos);
                TALLOC_FREE(req);
+
+               status = state.status;
+               if (NT_STATUS_EQUAL(status, NT_STATUS_NO_MORE_FILES)) {
+                       status = NT_STATUS_OK;
+               }
        } else {
                status = cli_list(self->cli, mask, attribute, callback_fn,
                                  priv);
        }
-       TALLOC_FREE(mask);
-
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
 
-       /* invoke the callback for the async results (SMBv1 connections) */
-       for (i = 0; i < num_finfos; i++) {
-               status = callback_fn(&finfos[i], user_mask,
-                                    priv);
-               if (!NT_STATUS_IS_OK(status)) {
-                       TALLOC_FREE(finfos);
-                       return status;
-               }
-       }
-
-       TALLOC_FREE(finfos);
+done:
+       TALLOC_FREE(mask);
        return status;
 }