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/10/25 23:11:29 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
yamt opened a new pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110
## Summary
include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
A similar fix for another implementation:
https://github.com/apache/incubator-nuttx/pull/1889
## Impact
## Testing
locally built and run for sim, with some other patches.
----------------------------------------------------------------
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] yamt closed pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
yamt closed pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110
----------------------------------------------------------------
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] yamt commented on a change in pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511697715
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
while i haven't investigated those usage deeply, i guess they are not "mandatory" parts of the library as helloxx/cxxtest are working for me.
----------------------------------------------------------------
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 #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511695424
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
should we always set CONFIG_STDIO_DISABLE_BUFFERING to n when CONFIG_LIBCXX equal y? since libcxx call setbuf in iostream class.
----------------------------------------------------------------
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 #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511709102
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
Since libcxx will increase at least 20KB code, I don't think that it's benefit to disable CONFIG_STDIO_DISABLE_BUFFERING
----------------------------------------------------------------
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 #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511745210
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
Why don't let the linker remove the unused code for us? and it's also strange to couple setbuf/setvbuf with CONFIG_LIBCXX.
----------------------------------------------------------------
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] btashton commented on a change in pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r512420812
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
@xiaoxiang781216 that PR is now merged.
----------------------------------------------------------------
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] yamt commented on pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#issuecomment-716987585
no longer necessary as https://github.com/apache/incubator-nuttx/pull/2119 is merged
----------------------------------------------------------------
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] yamt commented on a change in pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511755780
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
btw, is there a reasonable way to make "always set CONFIG_STDIO_DISABLE_BUFFERING to n when CONFIG_LIBCXX equal y" in Kconfig?
"select" looks similar but it doesn't seem to allow "select !STDIO_DISABLE_BUFFERING".
"depends on !STDIO_DISABLE_BUFFERING" sounds too strong for this in my taste.
----------------------------------------------------------------
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] yamt commented on a change in pull request #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511710854
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
while i don't know how much CONFIG_STDIO_DISABLE_BUFFERING can save, i guess every bytes can accumulate.
----------------------------------------------------------------
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 #2110: include/stdio.h: Fix build errors with CONFIG_LIBCXX=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2110:
URL: https://github.com/apache/incubator-nuttx/pull/2110#discussion_r511927044
##########
File path: include/stdio.h
##########
@@ -164,7 +164,7 @@ FAR char *gets(FAR char *s);
FAR char *gets_s(FAR char *s, rsize_t n);
void rewind(FAR FILE *stream);
-#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if !defined(CONFIG_STDIO_DISABLE_BUFFERING) || defined(CONFIG_LIBCXX)
Review comment:
How about this change:
https://github.com/apache/incubator-nuttx/pull/2119
since libc always provide a dummy implementaion:
https://github.com/apache/incubator-nuttx/blob/master/libs/libc/stdio/lib_setbuf.c#L85
https://github.com/apache/incubator-nuttx/blob/master/libs/libc/stdio/lib_setvbuf.c#L275
the correct fix is always define the prototype and include ib_setbuf.c/ib_setvbuf.c into 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