lib: fix error on MacOS #6032
Closed
lib: fix error on MacOS #6032
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
polychaeta
reviewed
Mar 18, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/3f3ea50f2f5f9bd11fd2ea4fbdc7dcc9/raw/07a8e684d5bc430e1b31623df4d2a1b23f720dcd/cr_6032_1584543107.diff | git apply
diff --git a/lib/memory.h b/lib/memory.h
index cf3390af4..f89a5c299 100644
--- a/lib/memory.h
+++ b/lib/memory.h
@@ -83,13 +83,12 @@ struct memgroup {
#define DECLARE_MGROUP(name) extern struct memgroup _mg_##name;
#define DEFINE_MGROUP(mname, desc) \
- struct memgroup _mg_##mname \
- ATTRIBUTE_MGROUPS = { \
- .name = desc, \
- .types = NULL, \
- .next = NULL, \
- .insert = NULL, \
- .ref = NULL, \
+ struct memgroup _mg_##mname ATTRIBUTE_MGROUPS = { \
+ .name = desc, \
+ .types = NULL, \
+ .next = NULL, \
+ .insert = NULL, \
+ .ref = NULL, \
}; \
static void _mginit_##mname(void) __attribute__((_CONSTRUCTOR(1000))); \
static void _mginit_##mname(void) \
@@ -119,14 +118,13 @@ struct memgroup {
#endif
#define DEFINE_MTYPE_ATTR(group, mname, attr, desc) \
- attr struct memtype MTYPE_##mname[1] \
- ATTRIBUTE_MTYPES = { { \
- .name = desc, \
- .next = NULL, \
- .n_alloc = 0, \
- .size = 0, \
- .ref = NULL, \
- } }; \
+ attr struct memtype MTYPE_##mname[1] ATTRIBUTE_MTYPES = {{ \
+ .name = desc, \
+ .next = NULL, \
+ .n_alloc = 0, \
+ .size = 0, \
+ .ref = NULL, \
+ }}; \
static void _mtinit_##mname(void) __attribute__((_CONSTRUCTOR(1001))); \
static void _mtinit_##mname(void) \
{ \
@@ -134,7 +132,7 @@ struct memgroup {
_mg_##group.insert = &_mg_##group.types; \
MTYPE_##mname->ref = _mg_##group.insert; \
*_mg_##group.insert = MTYPE_##mname; \
- _mg_##group.insert = &MTYPE_##mname->next; \
+ _mg_##group.insert = &MTYPE_##mname->next; \
} \
static void _mtfini_##mname(void) __attribute__((_DESTRUCTOR(1001))); \
static void _mtfini_##mname(void) \
If you are a new contributor to FRR, please see our contributing guidelines.
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)Checkout code: No useful log found |
|
Please note I tried to verify that these attributes actually add new segments into the binary, but they don't show up on Linux in the output of readelf -a, before or after this change. I can't find where we actually use this functionality (separate segment per MTYPE), so perhaps the better change here would be to remove the attributes altogether? @eqvinox what do you think? |
|
ci:rerun |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: FailedCheckout code: Failed (click for details)Checkout code: No useful log found |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotest tests on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPOI386-11266/test Topology Tests failed for Topotest tests on Ubuntu 16.04 i386: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11266/artifact/TOPOI386/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11265/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
ci:rerun |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11277/ This is a comment from an automated CI system. |
rubenk
force-pushed
the
lib-fix-attribute-macos
branch
from
973ec7c
to
a56ce75
Compare
polychaeta
reviewed
Mar 19, 2020
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/3d4d29a827ac1ab8ac227cd8a8a771d7/raw/07a8e684d5bc430e1b31623df4d2a1b23f720dcd/cr_6032_1584639330.diff | git apply
diff --git a/lib/memory.h b/lib/memory.h
index cf3390af4..f89a5c299 100644
--- a/lib/memory.h
+++ b/lib/memory.h
@@ -83,13 +83,12 @@ struct memgroup {
#define DECLARE_MGROUP(name) extern struct memgroup _mg_##name;
#define DEFINE_MGROUP(mname, desc) \
- struct memgroup _mg_##mname \
- ATTRIBUTE_MGROUPS = { \
- .name = desc, \
- .types = NULL, \
- .next = NULL, \
- .insert = NULL, \
- .ref = NULL, \
+ struct memgroup _mg_##mname ATTRIBUTE_MGROUPS = { \
+ .name = desc, \
+ .types = NULL, \
+ .next = NULL, \
+ .insert = NULL, \
+ .ref = NULL, \
}; \
static void _mginit_##mname(void) __attribute__((_CONSTRUCTOR(1000))); \
static void _mginit_##mname(void) \
@@ -119,14 +118,13 @@ struct memgroup {
#endif
#define DEFINE_MTYPE_ATTR(group, mname, attr, desc) \
- attr struct memtype MTYPE_##mname[1] \
- ATTRIBUTE_MTYPES = { { \
- .name = desc, \
- .next = NULL, \
- .n_alloc = 0, \
- .size = 0, \
- .ref = NULL, \
- } }; \
+ attr struct memtype MTYPE_##mname[1] ATTRIBUTE_MTYPES = {{ \
+ .name = desc, \
+ .next = NULL, \
+ .n_alloc = 0, \
+ .size = 0, \
+ .ref = NULL, \
+ }}; \
static void _mtinit_##mname(void) __attribute__((_CONSTRUCTOR(1001))); \
static void _mtinit_##mname(void) \
{ \
@@ -134,7 +132,7 @@ struct memgroup {
_mg_##group.insert = &_mg_##group.types; \
MTYPE_##mname->ref = _mg_##group.insert; \
*_mg_##group.insert = MTYPE_##mname; \
- _mg_##group.insert = &MTYPE_##mname->next; \
+ _mg_##group.insert = &MTYPE_##mname->next; \
} \
static void _mtfini_##mname(void) __attribute__((_DESTRUCTOR(1001))); \
static void _mtfini_##mname(void) \
If you are a new contributor to FRR, please see our contributing guidelines.
|
please apply the style suggestions |
Sections use a different syntax for Mach-O executables.
Fixes:
lib/bfd.c:35:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a
comma
DEFINE_MTYPE_STATIC(LIB, BFD_INFO, "BFD info")
^
./lib/memory.h:140:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc) \
^
./lib/memory.h:110:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
__attribute__((section(".data.mtypes"))) = { { \
^
1 error generated.
Signed-off-by: Ruben Kerkhof <ruben@rubenkerkhof.com>
rubenk
force-pushed
the
lib-fix-attribute-macos
branch
from
a56ce75
to
ad26e09
Compare
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11309/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-11315/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
eqvinox
requested changes
Mar 20, 2020
| #define ATTRIBUTE_MGROUPS __attribute__((section("__DATA,mgroups"))) | ||
| #else | ||
| #define ATTRIBUTE_MGROUPS __attribute__((section(".data.mgroups"))) | ||
| #endif |
There was a problem hiding this comment.
ACK, but
- this belongs in
lib/compiler.h - please make it one macro with a parameterized section name
#ifdef __MACH__
#define _DATA_SECTION(name) __attribute__((section("__DATA," name)))
#else
#define _DATA_SECTION(name) __attribute__((section(".data." name)))
#endif
(Note consecutive string literals in C are automatically concatenated, "foo" "bar" is the same as "foobar". This does work correctly for GCC attribute parameters.)
would be used like this:
struct memgroup _mg_##mname _DATA_SECTION("mgroups")
(we have some macros with the __attribute__ included in the macro, and some without the __attribute__ - I don't really care much either way on that... maybe we should clean that up and make it consistent one way or another.)
|
@rubenk are you planning to address the comments and update this PR? If not please let us know so we can give the work to someone else to finish up |
|
@polychaeta autoclose in 1 week |
Closed
httpstorm
added a commit
to httpstorm/openwrt.packages
that referenced
this pull request
Apr 30, 2024
Fixes:
lib/command_graph.c:16:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma
DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command Tokens");
^
./lib/memory.h:139:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc) \
^
./lib/memory.h:109:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
__attribute__((section(".data.mtypes"))) = { { \
[1] FRRouting/frr#6032
Signed-off-by: Georgi Valkov <gvalkov@gmail.com>
httpstorm
added a commit
to httpstorm/openwrt.packages
that referenced
this pull request
Apr 30, 2024
Fixes:
lib/command_graph.c:16:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma
DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command Tokens");
^
./lib/memory.h:139:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc) \
^
./lib/memory.h:109:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
__attribute__((section(".data.mtypes"))) = { { \
[1] FRRouting/frr#6032
Signed-off-by: Georgi Valkov <gvalkov@gmail.com>
httpstorm
added a commit
to httpstorm/openwrt.packages
that referenced
this pull request
Apr 30, 2024
Fixes:
lib/command_graph.c:16:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma
DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command Tokens");
^
./lib/memory.h:139:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc) \
^
./lib/memory.h:109:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
__attribute__((section(".data.mtypes"))) = { { \
[1] FRRouting/frr#6032
Signed-off-by: Georgi Valkov <gvalkov@gmail.com>
This was referenced Apr 30, 2024
httpstorm
added a commit
to httpstorm/openwrt.packages
that referenced
this pull request
Apr 30, 2024
Fixes:
lib/command_graph.c:16:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command Tokens"); ^
./lib/memory.h:139:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc) \
^
./lib/memory.h:109:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
__attribute__((section(".data.mtypes"))) = { { \
[1] FRRouting/frr#6032
[2] FRRouting/frr#15890
Signed-off-by: Georgi Valkov <gvalkov@gmail.com>
robimarko
pushed a commit
to openwrt/packages
that referenced
this pull request
Apr 30, 2024
Fixes:
lib/command_graph.c:16:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a comma DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command Tokens"); ^
./lib/memory.h:139:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc) \
^
./lib/memory.h:109:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
__attribute__((section(".data.mtypes"))) = { { \
[1] FRRouting/frr#6032
[2] FRRouting/frr#15890
Signed-off-by: Georgi Valkov <gvalkov@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sections use a different syntax for Mach-O executables.
Fixes:
lib/bfd.c:35:1: error: argument to 'section' attribute is not valid for this target: mach-o section specifier requires a segment and section separated by a
comma
DEFINE_MTYPE_STATIC(LIB, BFD_INFO, "BFD info")
^
./lib/memory.h:140:2: note: expanded from macro 'DEFINE_MTYPE_STATIC'
DEFINE_MTYPE_ATTR(group, name, static, desc)
^
./lib/memory.h:110:26: note: expanded from macro 'DEFINE_MTYPE_ATTR'
attribute((section(".data.mtypes"))) = { {
^
1 error generated.
Signed-off-by: Ruben Kerkhof ruben@rubenkerkhof.com