pkarashchenko commented on a change in pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844#discussion_r835784550
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -460,15 +496,34 @@ off_t rpmsgfs_client_lseek(FAR void *handle, int fd,
int rpmsgfs_client_ioctl(FAR void *handle, int fd,
int request, unsigned long arg)
{
- struct rpmsgfs_ioctl_s msg =
- {
- .fd = fd,
- .request = request,
- .arg = arg,
- };
+ size_t arglen = rpmsgfs_ioctl_arglen(request);
+ FAR struct rpmsgfs_s *priv = handle;
+ FAR struct rpmsgfs_ioctl_s *msg;
+ uint32_t space;
+ size_t len;
- return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, true,
- (struct rpmsgfs_header_s *)&msg, sizeof(msg), NULL);
+ len = sizeof(*msg) + arglen;
+ msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
+ if (!msg)
Review comment:
```suggestion
if (msg == NULL)
```
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -460,15 +496,34 @@ off_t rpmsgfs_client_lseek(FAR void *handle, int fd,
int rpmsgfs_client_ioctl(FAR void *handle, int fd,
int request, unsigned long arg)
{
- struct rpmsgfs_ioctl_s msg =
- {
- .fd = fd,
- .request = request,
- .arg = arg,
- };
+ size_t arglen = rpmsgfs_ioctl_arglen(request);
+ FAR struct rpmsgfs_s *priv = handle;
+ FAR struct rpmsgfs_ioctl_s *msg;
+ uint32_t space;
+ size_t len;
- return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, true,
- (struct rpmsgfs_header_s *)&msg, sizeof(msg), NULL);
+ len = sizeof(*msg) + arglen;
+ msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
+ if (!msg)
+ {
+ return -ENOMEM;
+ }
+
+ DEBUGASSERT(len <= space);
+
+ msg->fd = fd;
+ msg->request = request;
+ msg->arg = arg;
+ msg->arglen = arglen;
+
+ if (arglen > 0)
+ {
+ memcpy(msg->buf, (void *)(uintptr_t)arg, arglen);
+ }
+
+ return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, false,
+ (struct rpmsgfs_header_s *)msg, len,
+ arglen > 0 ? (void *)arg : NULL);
Review comment:
please add `FAR` to pointer casts
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -155,6 +159,25 @@ static int rpmsgfs_read_handler(FAR struct rpmsg_endpoint
*ept,
return 0;
}
+static int rpmsgfs_ioctl_handler(FAR struct rpmsg_endpoint *ept,
+ FAR void *data, size_t len,
+ uint32_t src, FAR void *priv)
+{
+ FAR struct rpmsgfs_header_s *header = data;
+ FAR struct rpmsgfs_cookie_s *cookie =
+ (struct rpmsgfs_cookie_s *)(uintptr_t)header->cookie;
Review comment:
```suggestion
(FAR struct rpmsgfs_cookie_s *)(uintptr_t)header->cookie;
```
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -155,6 +159,25 @@ static int rpmsgfs_read_handler(FAR struct rpmsg_endpoint
*ept,
return 0;
}
+static int rpmsgfs_ioctl_handler(FAR struct rpmsg_endpoint *ept,
+ FAR void *data, size_t len,
+ uint32_t src, FAR void *priv)
+{
+ FAR struct rpmsgfs_header_s *header = data;
+ FAR struct rpmsgfs_cookie_s *cookie =
+ (struct rpmsgfs_cookie_s *)(uintptr_t)header->cookie;
+ FAR struct rpmsgfs_ioctl_s *rsp = data;
+
+ if (cookie->result >= 0 && rsp->arglen > 0)
+ {
+ memcpy(cookie->data, (void *)(uintptr_t)rsp->arg, rsp->arglen);
Review comment:
```suggestion
memcpy(cookie->data, (FAR void *)(uintptr_t)rsp->arg, rsp->arglen);
```
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -460,15 +496,34 @@ off_t rpmsgfs_client_lseek(FAR void *handle, int fd,
int rpmsgfs_client_ioctl(FAR void *handle, int fd,
int request, unsigned long arg)
{
- struct rpmsgfs_ioctl_s msg =
- {
- .fd = fd,
- .request = request,
- .arg = arg,
- };
+ size_t arglen = rpmsgfs_ioctl_arglen(request);
+ FAR struct rpmsgfs_s *priv = handle;
+ FAR struct rpmsgfs_ioctl_s *msg;
+ uint32_t space;
+ size_t len;
- return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, true,
- (struct rpmsgfs_header_s *)&msg, sizeof(msg), NULL);
+ len = sizeof(*msg) + arglen;
+ msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
+ if (!msg)
+ {
+ return -ENOMEM;
+ }
+
+ DEBUGASSERT(len <= space);
+
+ msg->fd = fd;
+ msg->request = request;
+ msg->arg = arg;
+ msg->arglen = arglen;
+
+ if (arglen > 0)
+ {
+ memcpy(msg->buf, (void *)(uintptr_t)arg, arglen);
Review comment:
```suggestion
memcpy(msg->buf, (FAR void *)(uintptr_t)arg, arglen);
```
##########
File path: fs/rpmsgfs/rpmsgfs.h
##########
@@ -110,7 +110,9 @@ begin_packed_struct struct rpmsgfs_ioctl_s
struct rpmsgfs_header_s header;
int32_t fd;
int32_t request;
- int32_t arg;
+ uint64_t arg;
+ uint32_t arglen;
+ char buf[0];
Review comment:
@xiaoxiang781216 can we switch to true C99 `buf[];` instead of GCC
extension `buf[0];` if we really would like to use flexible array members in
the code?
--
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]
