libsmb: Change cli_posix_readlink to return talloc'ed target

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

diff --git a/source3/client/client.c b/source3/client/client.c
index 366368630cb1ce007c9f665cea2798b05387acae..a69ab5daa7631ac04aece6172d687ef2a774ab61 100644 (file)
--- a/source3/client/client.c
+++ b/source3/client/client.c
@@ -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;
 }
 

diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
index 8e157c83e994b6f4986cf52bebda32955a515f85..dec0dd016095f191dadb76993596595ef2a0394f 100644 (file)
--- a/source3/libsmb/clifile.c
+++ b/source3/libsmb/clifile.c
@@ -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);

diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index b488c051ed571171a47d7d272e5e5651d8e7a1c8..c811d07905f1b86a3ffa989848a1836e6d387c9e 100644 (file)
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -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,

diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index c7b3bf28041dd53ce183cb464b37061d89fe9461..219ac4a370cc04f859ba75f0d130f4a689091549 100644 (file)
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -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;
        }