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/02/24 07:34:03 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Donny9 opened a new pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904
## Summary
Enhance getcwd.
Reference: https://pubs.opengroup.org/onlinepubs/007904975/functions/getcwd.html
## Impact
## Testing
daily test
----------------------------------------------------------------
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] davids5 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581983966
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -63,6 +63,12 @@
* symbolic links. The 'size' argument is the size in bytes of the
* character array pointed to by the 'buf' argument.
*
+ * As an extension to the POSIX.1-2001 standard, getcwd() allocates
+ * the buffer dynamically using malloc if buf is NULL. In this case,
+ * the allocated buffer has the length size unless size is zero, when buf
Review comment:
the allocated buffer has the length size unless size is zero, when buf
the allocated buffer has the length size, unless size is zero, when buf
----------------------------------------------------------------
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] Donny9 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r582440816
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -63,6 +63,12 @@
* symbolic links. The 'size' argument is the size in bytes of the
* character array pointed to by the 'buf' argument.
*
+ * As an extension to the POSIX.1-2001 standard, getcwd() allocates
+ * the buffer dynamically using malloc if buf is NULL. In this case,
+ * the allocated buffer has the length size unless size is zero, when buf
Review comment:
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] davids5 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581863926
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -63,6 +63,12 @@
* symbolic links. The 'size' argument is the size in bytes of the
* character array pointed to by the 'buf' argument.
*
+ * As an extension to the POSIX.1-2001 standard, getcwd() allocates
+ * the buffer dynamically using malloc if buf is NULL. In this case,
+ * the allocated buffer has the length size unless size is zero, when buf
Review comment:
missing comma?
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -92,13 +98,18 @@ FAR char *getcwd(FAR char *buf, size_t size)
/* Verify input parameters */
#ifdef CONFIG_DEBUG_FEATURES
- if (!buf || !size)
+ if (buf && !size)
Review comment:
hmm why does is original code qualified by CONFIG_DEBUG_FEATURES?
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -92,13 +98,18 @@ FAR char *getcwd(FAR char *buf, size_t size)
/* Verify input parameters */
#ifdef CONFIG_DEBUG_FEATURES
- if (!buf || !size)
+ if (buf && !size)
{
set_errno(EINVAL);
return NULL;
}
#endif
+ if (!size)
Review comment:
```suggestion
if (size == 0)
```
----------------------------------------------------------------
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] Donny9 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581981061
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -63,6 +63,12 @@
* symbolic links. The 'size' argument is the size in bytes of the
* character array pointed to by the 'buf' argument.
*
+ * As an extension to the POSIX.1-2001 standard, getcwd() allocates
+ * the buffer dynamically using malloc if buf is NULL. In this case,
+ * the allocated buffer has the length size unless size is zero, when buf
Review comment:
what?
----------------------------------------------------------------
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] patacongo commented on pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#issuecomment-786117328
> @patacongo What is you take on this extension?
I don't have any strong opinion, as long as it is fully compatible with the require at the OpenGroup.org reference. We do not want any non-standard behavior in the system.
----------------------------------------------------------------
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] Donny9 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r583354679
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -115,6 +124,16 @@ FAR char *getcwd(FAR char *buf, size_t size)
return NULL;
}
+ if (buf == NULL)
+ {
+ buf = malloc(size);
Review comment:
Okay, thanks. 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] Donny9 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r582441378
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -92,13 +98,18 @@ FAR char *getcwd(FAR char *buf, size_t size)
/* Verify input parameters */
#ifdef CONFIG_DEBUG_FEATURES
- if (!buf || !size)
+ if (buf && !size)
Review comment:
okay， 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] davids5 merged pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904
----------------------------------------------------------------
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] davids5 commented on pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#issuecomment-786259137
> > @patacongo What is you take on this extension?
>
> I don't have any strong opinion, as long as it is fully compatible with the require at the OpenGroup.org reference. We do not want any non-standard behavior in the system.
OG says "If buf is a null pointer, the behavior of getcwd() is unspecified." Are you still good with this once the malloc is fixed?
----------------------------------------------------------------
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] Donny9 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581978696
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -92,13 +98,18 @@ FAR char *getcwd(FAR char *buf, size_t size)
/* Verify input parameters */
#ifdef CONFIG_DEBUG_FEATURES
- if (!buf || !size)
+ if (buf && !size)
{
set_errno(EINVAL);
return NULL;
}
#endif
+ if (!size)
Review comment:
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] davids5 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581984447
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -63,6 +63,12 @@
* symbolic links. The 'size' argument is the size in bytes of the
* character array pointed to by the 'buf' argument.
*
+ * As an extension to the POSIX.1-2001 standard, getcwd() allocates
+ * the buffer dynamically using malloc if buf is NULL. In this case,
+ * the allocated buffer has the length size unless size is zero, when buf
Review comment:
Or remove the extra space?
----------------------------------------------------------------
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 #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#issuecomment-786441497
> > > @patacongo What is you take on this extension?
> >
> >
> > I don't have any strong opinion, as long as it is fully compatible with the require at the OpenGroup.org reference. We do not want any non-standard behavior in the system.
>
> OG says "If buf is a null pointer, the behavior of getcwd() is unspecified." Are you still good with this once the malloc is fixed?
Yes, it's glibc specific behaviour. But, OG explicitly state that the behavior is unspecified which mean we can do any action here.
So we still confirm OG specification with this patch, but more compatible with the most used libc implementation.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r583075013
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -115,6 +124,16 @@ FAR char *getcwd(FAR char *buf, size_t size)
return NULL;
}
+ if (buf == NULL)
+ {
+ buf = malloc(size);
Review comment:
malloc() should not be used. Use lib_malloc() instead. That eliminates the problem case where this function is called from within the OS in certain build modes and malloc does not do what you would want it do do.
----------------------------------------------------------------
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] Donny9 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581982502
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -92,13 +98,18 @@ FAR char *getcwd(FAR char *buf, size_t size)
/* Verify input parameters */
#ifdef CONFIG_DEBUG_FEATURES
- if (!buf || !size)
+ if (buf && !size)
Review comment:
These parameters may be checked under DEBUG? I keep origin layout.
----------------------------------------------------------------
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] davids5 commented on a change in pull request #2904: unistd/getcwd: enhance getcwd when buf is NULL
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #2904:
URL: https://github.com/apache/incubator-nuttx/pull/2904#discussion_r581988931
##########
File path: libs/libc/unistd/lib_getcwd.c
##########
@@ -92,13 +98,18 @@ FAR char *getcwd(FAR char *buf, size_t size)
/* Verify input parameters */
#ifdef CONFIG_DEBUG_FEATURES
- if (!buf || !size)
+ if (buf && !size)
Review comment:
If it was a error message that makes sense not a return value. The function docs state the return values and is not conditional? Why not 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