You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/12/31 09:07:56 UTC
[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #2631: mqueue: simplify the mqueue reailize
GUIDINGLI opened a new pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631
## Summary
mqueue: simplify the mqueue reailize
1. remove description management in mqueue, save code size
2. use i_ops instead of i_mqueue to remove the dup logic
3. add file_mq_xx for kernel use
4. Obey open nx_open file_open rules
## Impact
All places where use mq,
The developer should be know:
user space: mq_xx (mq_open returned is FD, not a structure)
kernel space: file_mq_xx
## Testing
Testing pass on sim
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551013504
##########
File path: fs/mqueue/mq_open.c
##########
@@ -235,6 +262,115 @@ int nxmq_open(FAR const char *mq_name, int oflags, mode_t mode,
return ret;
}
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: file_mq_open
+ *
+ * Description:
+ * This function establish a connection between a named message queue and
+ * the calling task. This is an internal OS interface. It is functionally
+ * equivalent to mq_open() except that:
+ *
+ * - It is not a cancellation point, and
+ * - It does not modify the errno value.
+ *
+ * See comments with mq_open() for a more complete description of the
+ * behavior of this function
+ *
+ * Input Parameters:
+ * mq_name - Name of the queue to open
+ * oflags - open flags
+ * Optional parameters. When the O_CREAT flag is specified, two optional
+ * parameters are expected:
+ *
+ * 1. mode_t mode (ignored), and
+ * 2. struct mq_attr *attr. The mq_maxmsg attribute
+ * is used at the time that the message queue is
+ * created to determine the maximum number of
+ * messages that may be placed in the message queue.
+ *
+ * Returned Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success, mqdes point to the new message queue descriptor.
+ * A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+int file_mq_open(FAR struct file *mq,
+ FAR const char *mq_name, int oflags, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, oflags);
+ ret = file_mq_vopen(mq, mq_name, oflags, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxmq_open
+ *
+ * Description:
+ * This function establish a connection between a named message queue and
+ * the calling task. This is an internal OS interface. It is functionally
+ * equivalent to mq_open() except that:
+ *
+ * - It is not a cancellation point, and
+ * - It does not modify the errno value.
+ *
+ * See comments with mq_open() for a more complete description of the
+ * behavior of this function
+ *
+ * Input Parameters:
+ * mq_name - Name of the queue to open
+ * oflags - open flags
+ * Optional parameters. When the O_CREAT flag is specified, two optional
+ * parameters are expected:
+ *
+ * 1. mode_t mode (ignored), and
+ * 2. struct mq_attr *attr. The mq_maxmsg attribute
+ * is used at the time that the message queue is
+ * created to determine the maximum number of
+ * messages that may be placed in the message queue.
+ *
+ * Returned Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success, mqdes point to the new message queue descriptor.
+ * A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+int nxmq_open(FAR mqd_t *mqdes, FAR const char *mq_name, int oflags, ...)
+{
+ struct file mq;
+ va_list ap;
+ int ret;
+
+ va_start(ap, oflags);
+ ret = file_mq_vopen(&mq, mq_name, oflags, ap);
+ va_end(ap);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ *mqdes = files_allocate(mq.f_inode, mq.f_oflags, 0, 0);
Review comment:
f_pos ?
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#issuecomment-753287982
There is another issue when unlink is disabled, @liguiding please find a solution to fix it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#issuecomment-753608673
Aslo depends on:
https://github.com/apache/incubator-nuttx-apps/pull/546
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551101813
##########
File path: fs/mqueue/mq_unlink.c
##########
@@ -62,97 +62,43 @@
*
****************************************************************************/
-int nxmq_unlink(FAR const char *mq_name)
+int file_mq_unlink(FAR const char *mq_name)
Review comment:
Ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551013490
##########
File path: fs/mqueue/mq_unlink.c
##########
@@ -62,97 +62,43 @@
*
****************************************************************************/
-int nxmq_unlink(FAR const char *mq_name)
+int file_mq_unlink(FAR const char *mq_name)
Review comment:
you mean f_pos ?
If so, done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#issuecomment-752972220
This depends on https://github.com/apache/incubator-nuttx-apps/pull/544
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551013490
##########
File path: fs/mqueue/mq_unlink.c
##########
@@ -62,97 +62,43 @@
*
****************************************************************************/
-int nxmq_unlink(FAR const char *mq_name)
+int file_mq_unlink(FAR const char *mq_name)
Review comment:
kernel always ue file_mqxx, include file_mq_unlink ??
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551001295
##########
File path: fs/mqueue/mq_open.c
##########
@@ -235,6 +262,115 @@ int nxmq_open(FAR const char *mq_name, int oflags, mode_t mode,
return ret;
}
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: file_mq_open
+ *
+ * Description:
+ * This function establish a connection between a named message queue and
+ * the calling task. This is an internal OS interface. It is functionally
+ * equivalent to mq_open() except that:
+ *
+ * - It is not a cancellation point, and
+ * - It does not modify the errno value.
+ *
+ * See comments with mq_open() for a more complete description of the
+ * behavior of this function
+ *
+ * Input Parameters:
+ * mq_name - Name of the queue to open
+ * oflags - open flags
+ * Optional parameters. When the O_CREAT flag is specified, two optional
+ * parameters are expected:
+ *
+ * 1. mode_t mode (ignored), and
+ * 2. struct mq_attr *attr. The mq_maxmsg attribute
+ * is used at the time that the message queue is
+ * created to determine the maximum number of
+ * messages that may be placed in the message queue.
+ *
+ * Returned Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success, mqdes point to the new message queue descriptor.
+ * A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+int file_mq_open(FAR struct file *mq,
+ FAR const char *mq_name, int oflags, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start(ap, oflags);
+ ret = file_mq_vopen(mq, mq_name, oflags, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: nxmq_open
+ *
+ * Description:
+ * This function establish a connection between a named message queue and
+ * the calling task. This is an internal OS interface. It is functionally
+ * equivalent to mq_open() except that:
+ *
+ * - It is not a cancellation point, and
+ * - It does not modify the errno value.
+ *
+ * See comments with mq_open() for a more complete description of the
+ * behavior of this function
+ *
+ * Input Parameters:
+ * mq_name - Name of the queue to open
+ * oflags - open flags
+ * Optional parameters. When the O_CREAT flag is specified, two optional
+ * parameters are expected:
+ *
+ * 1. mode_t mode (ignored), and
+ * 2. struct mq_attr *attr. The mq_maxmsg attribute
+ * is used at the time that the message queue is
+ * created to determine the maximum number of
+ * messages that may be placed in the message queue.
+ *
+ * Returned Value:
+ * This is an internal OS interface and should not be used by applications.
+ * It follows the NuttX internal error return policy: Zero (OK) is
+ * returned on success, mqdes point to the new message queue descriptor.
+ * A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+int nxmq_open(FAR mqd_t *mqdes, FAR const char *mq_name, int oflags, ...)
+{
+ struct file mq;
+ va_list ap;
+ int ret;
+
+ va_start(ap, oflags);
+ ret = file_mq_vopen(&mq, mq_name, oflags, ap);
+ va_end(ap);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ *mqdes = files_allocate(mq.f_inode, mq.f_oflags, 0, 0);
Review comment:
change the first 0 to mq.f_priv
##########
File path: fs/mqueue/mq_unlink.c
##########
@@ -62,97 +62,43 @@
*
****************************************************************************/
-int nxmq_unlink(FAR const char *mq_name)
+int file_mq_unlink(FAR const char *mq_name)
Review comment:
don't need
##########
File path: fs/mqueue/mq_open.c
##########
@@ -266,31 +402,26 @@ int nxmq_open(FAR const char *mq_name, int oflags, mode_t mode,
mqd_t mq_open(FAR const char *mq_name, int oflags, ...)
{
- FAR struct mq_attr *attr = NULL;
- mode_t mode = 0;
+ struct file mq;
mqd_t mqdes;
va_list ap;
int ret;
- /* Were we asked to create it? */
-
- if ((oflags & O_CREAT) != 0)
+ va_start(ap, oflags);
+ ret = file_mq_vopen(&mq, mq_name, oflags, ap);
+ va_end(ap);
+ if (ret < 0)
{
- /* We have to extract the additional
- * parameters from the variable argument list.
- */
-
- va_start(ap, oflags);
- mode = va_arg(ap, mode_t);
- attr = va_arg(ap, FAR struct mq_attr *);
- va_end(ap);
+ set_errno(-ret);
+ return ERROR;
}
- ret = nxmq_open(mq_name, oflags, mode, attr, &mqdes);
- if (ret < 0)
+ mqdes = files_allocate(mq.f_inode, mq.f_oflags, 0, 0);
Review comment:
change the first 0 to mq.f_priv
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#issuecomment-950495564
@GUIDINGLI
I noticed that this PR caused a memory leak of 96 bytes when testing with nxplayer.
Could you please check what is happening ?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#issuecomment-753107941
The app PR is in I am restarting this build.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551013560
##########
File path: fs/mqueue/mq_open.c
##########
@@ -266,31 +402,26 @@ int nxmq_open(FAR const char *mq_name, int oflags, mode_t mode,
mqd_t mq_open(FAR const char *mq_name, int oflags, ...)
{
- FAR struct mq_attr *attr = NULL;
- mode_t mode = 0;
+ struct file mq;
mqd_t mqdes;
va_list ap;
int ret;
- /* Were we asked to create it? */
-
- if ((oflags & O_CREAT) != 0)
+ va_start(ap, oflags);
+ ret = file_mq_vopen(&mq, mq_name, oflags, ap);
+ va_end(ap);
+ if (ret < 0)
{
- /* We have to extract the additional
- * parameters from the variable argument list.
- */
-
- va_start(ap, oflags);
- mode = va_arg(ap, mode_t);
- attr = va_arg(ap, FAR struct mq_attr *);
- va_end(ap);
+ set_errno(-ret);
+ return ERROR;
}
- ret = nxmq_open(mq_name, oflags, mode, attr, &mqdes);
- if (ret < 0)
+ mqdes = files_allocate(mq.f_inode, mq.f_oflags, 0, 0);
Review comment:
f_pos ?
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2631: mqueue: simplify the mqueue reailize
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2631:
URL: https://github.com/apache/incubator-nuttx/pull/2631#discussion_r551271645
##########
File path: fs/mqueue/mq_open.c
##########
@@ -196,19 +241,18 @@ int nxmq_open(FAR const char *mq_name, int oflags, mode_t mode,
goto errout_with_inode;
}
- /* Create a message queue descriptor for the TCB */
+ /* Associate the inode with a file structure */
- *mqdes = nxmq_create_des(NULL, msgq, oflags);
- if (!*mqdes)
+ *mqdes = files_allocate(inode, oflags, 0, 0);
+ if (*mqdes < 0)
{
- ret = -ENOMEM;
+ ret = -EMFILE;
Review comment:
ret = *mqdes
##########
File path: fs/mqueue/mq_open.c
##########
@@ -147,14 +193,13 @@ int nxmq_open(FAR const char *mq_name, int oflags, mode_t mode,
goto errout_with_inode;
}
- /* Create a message queue descriptor for the current thread */
+ /* Associate the inode with a file structure */
- msgq = inode->u.i_mqueue;
- *mqdes = nxmq_create_des(NULL, msgq, oflags);
- if (!*mqdes)
+ *mqdes = files_allocate(inode, oflags, 0, 0);
+ if (*mqdes < 0)
{
- ret = -ENOMEM;
- goto errout_with_inode;
+ ret = -EMFILE;
Review comment:
change to:
```
ret = *mqdes
```
see https://github.com/apache/incubator-nuttx/pull/2639/files#diff-5adc55f87002f2d706285b3f201b7616f0055e9da6de5066f2f8bdcdab19d88fR63
##########
File path: fs/vfs/fs_unlink.c
##########
@@ -129,7 +129,12 @@ int nx_unlink(FAR const char *pathname)
* release all resources because it is no longer accessible.
*/
+#ifdef CONFIG_DISABLE_MQUEUE
if (INODE_IS_DRIVER(inode) && inode->u.i_ops->unlink)
+#else
+ if ((INODE_IS_DRIVER(inode) || INODE_IS_MQUEUE(inode))
+ && inode->u.i_ops->unlink)
Review comment:
don't need
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org