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 2021/10/24 23:13:27 UTC
[GitHub] [incubator-nuttx] masayuki2009 opened a new pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
masayuki2009 opened a new pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720
## Summary
- I noticed that spresense:rndis_smp crashed with stress test
- Finally, I found that it consumed more stack
- This commit fixes this issue by allocating fullpath on the heap
## Impact
- None
## Testing
- Tested with spresense:rndis_smp
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oops. PATH_MAX is 4Kb. NAME_MAX is 255B.
--
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] xiaoxiang781216 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735220475
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
MAX_MQUEUE_PATH is just 64 bytes, is it good to allocate a small temp buffer every time?
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
I shouldn't respond to things without doing the research...
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oops. PATH_MAX is 4Kb. NAME_MAX is 255B. These are configurable in NuttX, but default to 256 and 32, respectively.
sizeof(CONFIG_FS_MQUEUE_MPATH) defaults to 12 (Ignore my now deleted comment about this value being an integer, it is a string.).
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
I shouldn't respond to things without doing the research...
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oops. PATH_MAX is 4Kb. NAME_MAX is 255B. These are configurable in NuttX, but default to 256 and 32, respectively.
sizeof(CONFIG_FS_MQUEUE_MPATH) defaults to 11 (Ignore my now deleted comment about this value being an integer, it is a string.).
--
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] masayuki2009 closed pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
masayuki2009 closed pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735252917
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> I concern not only the speed, but also the memory fragment.
These memory allocations are *very* brief. I don't think that fragmentation would be an serious issue in this case. Small allocations that persist are a fragmentation problem, but not so much for transient allocations.
--
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] xiaoxiang781216 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735275266
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> `#define MAX_MQUEUE_PATH 64`
>
> @xiaoxiang781216 The above change is fine for me but do you know the intention of PR #4698?
The change want to report the accurate error number.
--
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] masayuki2009 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735285534
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
@xiaoxiang781216
>The change want to report the accurate error number.
OK. I think we should change the path length check as well.
Anyway, I'll create another PR.
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735231880
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
In #4698
#define MAX_MQUEUE_PATH (sizeof(CONFIG_FS_MQUEUE_MPATH) + NAME_MAX)
First, the use of sizeof on an integer constant seems a little odd and probably not portable.
Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
> MAX_MQUEUE_PATH is just 64 bytes, is it good to allocate a small temp buffer every time?'
The message queue implementation uses pre-allocated data as much as possible for performance reasons; messaging operations must be very fast. But this stack allocation is in fs_vopen(). I would not think that an allocation while opening a named message queue would have any performance implication since that is a rare event.
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735231880
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
In #4698
#define MAX_MQUEUE_PATH (sizeof(CONFIG_FS_MQUEUE_MPATH) + NAME_MAX)
Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
> MAX_MQUEUE_PATH is just 64 bytes, is it good to allocate a small temp buffer every time?'
The message queue implementation uses pre-allocated data as much as possible for performance reasons; messaging operations must be very fast. But this stack allocation is in fs_vopen(). I would not think that an allocation while opening a named message queue would have any performance implication since that is a rare event.
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oopa. PATH_MAX is 4Kb. NAME_MAX is 255.
--
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] masayuki2009 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735222613
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
Please see https://github.com/apache/incubator-nuttx/pull/4698 which was merged recently.
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oops. PATH_MAX is 4Kb. NAME_MAX is 255B. These are configurable in NuttX, but default to 256 and 32, respectively.
--
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] xiaoxiang781216 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735234968
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
But, that change doesn't increase the stack usage with the default config?
```
#define MAX_MQUEUE_PATH (sizeof(CONFIG_FS_MQUEUE_MPATH) + NAME_MAX)
#define CONFIG_FS_MQUEUE_MPATH "/var/mqueue"
#define CONFIG_NAME_MAX 32
```
MAX_MQUEUE_PATH equals 44(12 + 32).
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oops. PATH_MAX is 4Kb. NAME_MAX is 255B. These are configurable in NuttX, but default to 256 and 32, respectively.
sizeof(CONFIG_FS_MQUEUE_MPATH) defaults to 11 (Ignore my now deleted comment about this value being an integer, it is a string.).
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735236998
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> But, that change doesn't increase the stack usage with the default config?
True, but the size is configurable and not bounded. It would create a difficult to fix bug is a large value were configured. Isn't it better to idiot-proof 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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] masayuki2009 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735252823
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
`#define MAX_MQUEUE_PATH 64`
@xiaoxiang781216
The above change is fine for me but do you know the intention of PR #4698?
--
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] xiaoxiang781216 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735249214
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
If the bounded stack is a concern, it would be better to keep the original definition:
```
#define MAX_MQUEUE_PATH 64
```
which is more simple and work in the most case.
--
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] xiaoxiang781216 commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735248060
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> The message queue implementation uses pre-allocated data as much as possible for performance reasons; messaging operations must be very fast. But this stack allocation is in fs_vopen(). I would not think that an allocation while opening a named message queue would have any performance implication since that is a rare event.
I concern not only the speed, but also the memory fragment.
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735233407
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> Under GNU/Linux, I recall that NAME_MAX is 4Kb. If someone needed GNU/Linux compatibility, then that would be quite large.
Oops. PATH_MAX is 4Kb. NAME_MAX is 255.
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735237699
##########
File path: fs/mqueue/mq_open.c
##########
@@ -206,6 +206,14 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
mq_name++;
}
+ fullpath = (FAR char *)kmm_malloc(MAX_MQUEUE_PATH);
+
Review comment:
Wouldn't asprintf() simplify all of this?
--
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] patacongo commented on a change in pull request #4720: fs: mqueue: Allocate fullpath on the heap memory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #4720:
URL: https://github.com/apache/incubator-nuttx/pull/4720#discussion_r735236998
##########
File path: fs/mqueue/mq_open.c
##########
@@ -165,7 +165,7 @@ static int file_mq_vopen(FAR struct file *mq, FAR const char *mq_name,
FAR struct mqueue_inode_s *msgq;
FAR struct mq_attr *attr = NULL;
struct inode_search_s desc;
- char fullpath[MAX_MQUEUE_PATH];
Review comment:
> But, that change doesn't increase the stack usage with the default config?
True, but the size is configurable and not bounded. It would create a difficult to fix bug is a large value were configured. Isn't it better to idiot-proof the code?
A stack overflow can have mysterious symptoms and would be an unexpected consequence of increasing the sizes in the configuration.
--
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