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/04/13 03:51:35 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #775: modlib: #if
0 out unused modlib_sectname
yamt opened a new pull request #775: modlib: #if 0 out unused modlib_sectname
URL: https://github.com/apache/incubator-nuttx/pull/775
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #775:
Enable modlib_findsection
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #775: Enable modlib_findsection
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407534435
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
isn't "Nutt" of "NuttX" your name? i guess it will be there ~forever.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #775:
modlib: #if 0 out unused modlib_sectname
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #775: modlib: #if 0 out unused modlib_sectname
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407351042
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
@patacongo i think you added #if 0 on modlib_findsection. do you have any opinion?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #775:
modlib: #if 0 out unused modlib_sectname
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #775: modlib: #if 0 out unused modlib_sectname
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407335947
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
i just followed the existing convention. i'm fine with either ways.
iirc, the linker optimization traditionally works at object level, not functions, though.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request
#775: Enable modlib_findsection
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #775: Enable modlib_findsection
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407508184
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
> i think you added #if 0 on modlib_findsection. do you have any opinion?
If it isn't used, why waste memory.
It seems to me that there are two global functions in that file that are not closely related. I think they should pull out into separate files. I would do this:
1) Move modlib_loadshdrs() into a new file. I don't understand why it is in that file, and
2) Remove the #if 0
Then modlib_findsection() will be brought into build only if it is used with no dependency to lto.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo merged pull request #775: Enable
modlib_findsection
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #775: Enable modlib_findsection
URL: https://github.com/apache/incubator-nuttx/pull/775
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request
#775: Enable modlib_findsection
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #775: Enable modlib_findsection
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407527827
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
I don't see any reason why modlib_loadshdrs() and modlib_findsection() are in the same file. Do you see some dependency? They there is not some shared dependency or very close functional releationship, they belong in separate files.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #775: modlib: #if 0 out unused modlib_sectname
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #775: modlib: #if 0 out unused modlib_sectname
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407417012
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
> i just followed the existing convention. i'm fine with either ways.
> iirc, the linker optimization traditionally works at object level, not functions, though.
Yes, we may need to enable gc, but if user take care memory consumption, he should eanble gc anyway.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #775:
Enable modlib_findsection
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #775: Enable modlib_findsection
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407528100
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
@patacongo a good idea. done.
i also replaced your copyright notices in this directory with Apache 2.0 as i added a new file with the same license.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #775: modlib: #if 0 out unused modlib_sectname
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #775: modlib: #if 0 out unused modlib_sectname
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407325616
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
How about we remove #if 0 from modlib_findsection? The compiler/linker will optimize out the unused function.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request
#775: Enable modlib_findsection
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #775: Enable modlib_findsection
URL: https://github.com/apache/incubator-nuttx/pull/775#discussion_r407530326
##########
File path: libs/libc/modlib/modlib_sections.c
##########
@@ -66,6 +66,7 @@
*
****************************************************************************/
+#if 0 /* Not used */
Review comment:
Thanks! Pretty soon, my name won't be on any of the code. I have mixed feelings about that after almost 25 years of working with this code (and daily for the last 15 years) with over 36 thousand commits. But it is the best for the future.
----------------------------------------------------------------
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
With regards,
Apache Git Services