libsmb: Change cli_posix_readlink to return talloc'ed target
authorVolker Lendecke <vl@samba.org>
Tue, 26 Mar 2019 08:48:16 +0000 (09:48 +0100)
committerJeremy Allison <jra@samba.org>
Wed, 27 Mar 2019 12:31:37 +0000 (12:31 +0000)
This is a deviation from the Posix readlink function that from my
point of view makes this function easier to use. In Posix, probably
the assumption is that readlink is cheap, so someone under memory
constraints could just start with a small buffer and incrementally
increase the buffer size. For us, it's a network round-trip, and we
have the luxury of [mt]alloc, which the syscall kernel interface does
not have.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Wed Mar 27 12:31:37 UTC 2019 on sn-devel-144

source3/client/client.c
source3/libsmb/clifile.c
source3/libsmb/proto.h
source3/torture/torture.c

index 3663686..a69ab5d 100644 (file)
@@ -3487,7 +3487,7 @@ static int cmd_readlink(void)
        char *name= NULL;
        char *buf = NULL;
        char *targetname = NULL;
-       char linkname[PATH_MAX+1];
+       char *linkname = NULL;
        struct cli_state *targetcli;
         NTSTATUS status;
 
@@ -3519,7 +3519,7 @@ static int cmd_readlink(void)
                return 1;
        }
 
-       status = cli_posix_readlink(targetcli, name, linkname, PATH_MAX+1);
+       status = cli_posix_readlink(targetcli, name, talloc_tos(), &linkname);
        if (!NT_STATUS_IS_OK(status)) {
                d_printf("%s readlink on file %s\n",
                         nt_errstr(status), name);
@@ -3528,6 +3528,8 @@ static int cmd_readlink(void)
 
        d_printf("%s -> %s\n", name, linkname);
 
+       TALLOC_FREE(linkname);
+
        return 0;
 }
 
index 8e157c8..dec0dd0 100644 (file)
@@ -301,8 +301,7 @@ NTSTATUS cli_posix_symlink(struct cli_state *cli,
 
 struct cli_posix_readlink_state {
        struct cli_state *cli;
-       uint8_t *data;
-       uint32_t num_data;
+       char *converted;
 };
 
 static void cli_posix_readlink_done(struct tevent_req *subreq);
@@ -310,12 +309,10 @@ static void cli_posix_readlink_done(struct tevent_req *subreq);
 struct tevent_req *cli_posix_readlink_send(TALLOC_CTX *mem_ctx,
                                        struct tevent_context *ev,
                                        struct cli_state *cli,
-                                       const char *fname,
-                                       size_t len)
+                                       const char *fname)
 {
        struct tevent_req *req = NULL, *subreq = NULL;
        struct cli_posix_readlink_state *state = NULL;
-       uint32_t maxbytelen = (uint32_t)(smbXcli_conn_use_unicode(cli->conn) ? len*3 : len);
 
        req = tevent_req_create(
                mem_ctx, &state, struct cli_posix_readlink_state);
@@ -324,16 +321,14 @@ struct tevent_req *cli_posix_readlink_send(TALLOC_CTX *mem_ctx,
        }
        state->cli = cli;
 
-       /*
-        * Len is in bytes, we need it in UCS2 units.
-        */
-       if ((2*len < len) || (maxbytelen < len)) {
-               tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
-               return tevent_req_post(req, ev);
-       }
-
-       subreq = cli_qpathinfo_send(state, ev, cli, fname,
-                                   SMB_QUERY_FILE_UNIX_LINK, 1, maxbytelen);
+       subreq = cli_qpathinfo_send(
+               state,
+               ev,
+               cli,
+               fname,
+               SMB_QUERY_FILE_UNIX_LINK,
+               1,
+               UINT16_MAX);
        if (tevent_req_nomem(subreq, req)) {
                return tevent_req_post(req, ev);
        }
@@ -348,9 +343,13 @@ static void cli_posix_readlink_done(struct tevent_req *subreq)
        struct cli_posix_readlink_state *state = tevent_req_data(
                req, struct cli_posix_readlink_state);
        NTSTATUS status;
+       uint8_t *data = NULL;
+       uint32_t num_data;
+       charset_t charset;
+       size_t converted_size;
+       bool ok;
 
-       status = cli_qpathinfo_recv(subreq, state, &state->data,
-                                   &state->num_data);
+       status = cli_qpathinfo_recv(subreq, state, &data, &num_data);
        TALLOC_FREE(subreq);
        if (tevent_req_nterror(req, status)) {
                return;
@@ -358,49 +357,49 @@ static void cli_posix_readlink_done(struct tevent_req *subreq)
        /*
         * num_data is > 1, we've given 1 as minimum to cli_qpathinfo_send
         */
-       if (state->data[state->num_data-1] != '\0') {
+       if (data[num_data-1] != '\0') {
                tevent_req_nterror(req, NT_STATUS_DATA_ERROR);
                return;
        }
+
+       charset = smbXcli_conn_use_unicode(state->cli->conn) ?
+               CH_UTF16LE : CH_DOS;
+
+       /* The returned data is a pushed string, not raw data. */
+       ok = convert_string_talloc(
+               state,
+               charset,
+               CH_UNIX,
+               data,
+               num_data,
+               &state->converted,
+               &converted_size);
+       if (!ok) {
+               tevent_req_oom(req);
+               return;
+       }
        tevent_req_done(req);
 }
 
 NTSTATUS cli_posix_readlink_recv(
-       struct tevent_req *req, char *retpath, size_t len)
+       struct tevent_req *req, TALLOC_CTX *mem_ctx, char **target)
 {
        struct cli_posix_readlink_state *state = tevent_req_data(
                req, struct cli_posix_readlink_state);
        NTSTATUS status;
-       char *converted = NULL;
-       size_t converted_size = 0;
-       bool ok;
 
        if (tevent_req_is_nterror(req, &status)) {
                return status;
        }
-       /* The returned data is a pushed string, not raw data. */
-       ok = convert_string_talloc(
-               state,
-               smbXcli_conn_use_unicode(state->cli->conn) ? CH_UTF16LE:CH_DOS,
-               CH_UNIX,
-               state->data,
-               state->num_data,
-               &converted,
-               &converted_size);
-       if (!ok) {
-               return NT_STATUS_NO_MEMORY;
-       }
-
-       len = MIN(len,converted_size);
-       if (len == 0) {
-               return NT_STATUS_DATA_ERROR;
-       }
-       memcpy(retpath, converted, len);
+       *target = talloc_move(mem_ctx, &state->converted);
        return NT_STATUS_OK;
 }
 
-NTSTATUS cli_posix_readlink(struct cli_state *cli, const char *fname,
-                               char *linkpath, size_t len)
+NTSTATUS cli_posix_readlink(
+       struct cli_state *cli,
+       const char *fname,
+       TALLOC_CTX *mem_ctx,
+       char **target)
 {
        TALLOC_CTX *frame = talloc_stackframe();
        struct tevent_context *ev = NULL;
@@ -421,11 +420,7 @@ NTSTATUS cli_posix_readlink(struct cli_state *cli, const char *fname,
                goto fail;
        }
 
-       req = cli_posix_readlink_send(frame,
-                               ev,
-                               cli,
-                               fname,
-                               len);
+       req = cli_posix_readlink_send(frame, ev, cli, fname);
        if (req == NULL) {
                status = NT_STATUS_NO_MEMORY;
                goto fail;
@@ -435,7 +430,7 @@ NTSTATUS cli_posix_readlink(struct cli_state *cli, const char *fname,
                goto fail;
        }
 
-       status = cli_posix_readlink_recv(req, linkpath, len);
+       status = cli_posix_readlink_recv(req, mem_ctx, target);
 
  fail:
        TALLOC_FREE(frame);
index b488c05..c811d07 100644 (file)
@@ -265,12 +265,14 @@ NTSTATUS cli_posix_symlink(struct cli_state *cli,
 struct tevent_req *cli_posix_readlink_send(TALLOC_CTX *mem_ctx,
                                        struct tevent_context *ev,
                                        struct cli_state *cli,
-                                       const char *fname,
-                                       size_t len);
+                                       const char *fname);
 NTSTATUS cli_posix_readlink_recv(
-       struct tevent_req *req, char *retpath, size_t len);
-NTSTATUS cli_posix_readlink(struct cli_state *cli, const char *fname,
-                       char *linkpath, size_t len);
+       struct tevent_req *req, TALLOC_CTX *mem_ctx, char **target);
+NTSTATUS cli_posix_readlink(
+       struct cli_state *cli,
+       const char *fname,
+       TALLOC_CTX *mem_ctx,
+       char **target);
 struct tevent_req *cli_posix_hardlink_send(TALLOC_CTX *mem_ctx,
                                        struct tevent_context *ev,
                                        struct cli_state *cli,
index c7b3bf2..219ac4a 100644 (file)
@@ -6228,7 +6228,7 @@ static bool run_simple_posix_open_test(int dummy)
        const char *sname = "posix:symlink";
        const char *dname = "posix:dir";
        char buf[10];
-       char namebuf[11];
+       char *target = NULL;
        uint16_t fnum1 = (uint16_t)-1;
        SMB_STRUCT_STAT sbuf;
        bool correct = false;
@@ -6515,15 +6515,15 @@ static bool run_simple_posix_open_test(int dummy)
                }
        }
 
-       status = cli_posix_readlink(cli1, sname, namebuf, sizeof(namebuf));
+       status = cli_posix_readlink(cli1, sname, talloc_tos(), &target);
        if (!NT_STATUS_IS_OK(status)) {
                printf("POSIX readlink on %s failed (%s)\n", sname, nt_errstr(status));
                goto out;
        }
 
-       if (strcmp(namebuf, fname) != 0) {
+       if (strcmp(target, fname) != 0) {
                printf("POSIX readlink on %s failed to match name %s (read %s)\n",
-                       sname, fname, namebuf);
+                       sname, fname, target);
                goto out;
        }