xiaoxiang781216 commented on code in PR #20593:
URL: https://github.com/apache/nuttx/pull/20593#discussion_r4244492904


##########
fs/v9fs/client.c:
##########
@@ -2381,38 +2374,56 @@ ssize_t v9fs_client_readlink(FAR struct v9fs_client_s 
*client, uint32_t fid,
                              FAR char *buffer, size_t buflen)
 {
   struct v9fs_readlink_s request;
-  struct v9fs_rreadlink_s response;
+  FAR char *response;
   struct iovec wiov[1];
   struct iovec riov[1];
+  uint16_t len;
   int ret;
 
   /* size[4] Treadlink tag[2] fid[4]
    * size[4] Rreadlink tag[2] target[s]
+   *
+   * The response is too big for the stack: use a path buffer.
    */
 
+  response = lib_get_pathbuffer();

Review Comment:
   call lib_get_tempbuffer instead



##########
fs/v9fs/client.c:
##########
@@ -2381,38 +2374,56 @@ ssize_t v9fs_client_readlink(FAR struct v9fs_client_s 
*client, uint32_t fid,
                              FAR char *buffer, size_t buflen)
 {
   struct v9fs_readlink_s request;
-  struct v9fs_rreadlink_s response;
+  FAR char *response;
   struct iovec wiov[1];
   struct iovec riov[1];
+  uint16_t len;
   int ret;
 
   /* size[4] Treadlink tag[2] fid[4]
    * size[4] Rreadlink tag[2] target[s]
+   *
+   * The response is too big for the stack: use a path buffer.
    */
 
+  response = lib_get_pathbuffer();
+  if (response == NULL)
+    {
+      return -ENOMEM;
+    }
+
   request.fid = fid;
   request.header.size = V9FS_HDRSZ + V9FS_BIT32SZ;
   request.header.type = V9FS_TREADLINK;
   request.header.tag = v9fs_get_tagid(client);
 
   wiov[0].iov_base = &request;
   wiov[0].iov_len = V9FS_HDRSZ + V9FS_BIT32SZ;
-  riov[0].iov_base = &response;
-  riov[0].iov_len = sizeof(response);
+  riov[0].iov_base = response;
+  riov[0].iov_len = PATH_MAX;
 
   ret = v9fs_client_rpc(client->transport, wiov, 1, riov, 1,
                         request.header.tag);
   if (ret < 0)
     {
-      return ret;
+      goto out;
     }
 
-  if (response.target_len > buflen)
+  memcpy(&len, &response[V9FS_HDRSZ], V9FS_BIT16SZ);

Review Comment:
   should we keep v9fs_rreadlink_s, but change it to:
   ```
   begin_packed_struct struct v9fs_rreadlink_s
   {
     struct v9fs_header_s header;
     uint16_t target_len;
     uint8_t target[];
   } end_packed_struct;
   ```
   to simplify the following code.



##########
fs/v9fs/client.c:
##########
@@ -2293,42 +2293,61 @@ int v9fs_client_symlink(FAR struct v9fs_client_s 
*client, uint32_t fid,
                         FAR const char *name, FAR const char *symtgt,
                         int mode)
 {
-  struct v9fs_symlink_s request;
+  FAR struct v9fs_symlink_s *request;
   struct v9fs_rsymlink_s response;
   struct iovec wiov[1];
   struct iovec riov[1];
   uint32_t gid = getgid();
   off_t offset = 0;
   uint16_t symtgt_len;
+  size_t name_len;
+  uint16_t tag;
+  int ret;
 
   /* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4]
    * size[4] Rsymlink tag[2] qid[13]
    */
 
-  request.fid = fid;
-  request.name_len = strlen(name);
-  symtgt_len = strlen(symtgt);
-  request.header.size = V9FS_HDRSZ + V9FS_BIT32SZ * 2 + V9FS_BIT16SZ * 2 +
-                        request.name_len + symtgt_len;
-  request.header.type = V9FS_TSYMLINK;
-  request.header.tag = v9fs_get_tagid(client);
+  name_len = strlen(name);
+  symtgt_len = strnlen(symtgt, PATH_MAX);
+  if (name_len > NAME_MAX || symtgt_len >= PATH_MAX)
+    {
+      return -ENAMETOOLONG;
+    }
 
-  memcpy(&request.buffer[offset], name, request.name_len);
-  offset += request.name_len;
-  memcpy(&request.buffer[offset], &symtgt_len, V9FS_BIT16SZ);
+  /* The request is too big for the stack */
+
+  request = (FAR struct v9fs_symlink_s *)
+            lib_get_tempbuffer(sizeof(struct v9fs_symlink_s));
+  if (request == NULL)
+    {
+      return -ENOMEM;
+    }
+
+  tag = v9fs_get_tagid(client);
+  request->fid = fid;
+  request->name_len = name_len;
+  request->header.size = V9FS_HDRSZ + V9FS_BIT32SZ * 2 +
+                         V9FS_BIT16SZ * 2 + name_len + symtgt_len;
+  request->header.type = V9FS_TSYMLINK;
+  request->header.tag = tag;
+
+  memcpy(&request->buffer[offset], name, name_len);
+  offset += name_len;
+  memcpy(&request->buffer[offset], &symtgt_len, V9FS_BIT16SZ);
   offset += V9FS_BIT16SZ;
-  memcpy(&request.buffer[offset], symtgt, symtgt_len);
+  memcpy(&request->buffer[offset], symtgt, symtgt_len);
   offset += symtgt_len;
-  memcpy(&request.buffer[offset], &gid, V9FS_BIT32SZ);
+  memcpy(&request->buffer[offset], &gid, V9FS_BIT32SZ);
 
-  wiov[0].iov_base = &request;
-  wiov[0].iov_len = V9FS_HDRSZ + V9FS_BIT32SZ * 2 + V9FS_BIT16SZ * 2 +
-                    request.name_len + symtgt_len;
+  wiov[0].iov_base = request;
+  wiov[0].iov_len = request->header.size;
   riov[0].iov_base = &response;
   riov[0].iov_len = V9FS_HDRSZ + V9FS_QIDSZ;
 
-  return v9fs_client_rpc(client->transport, wiov, 1, riov, 1,
-                         request.header.tag);
+  ret = v9fs_client_rpc(client->transport, wiov, 1, riov, 1, tag);
+  lib_put_tempbuffer((FAR char *)request);

Review Comment:
   remove the cast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to