Function-based Lua hook call #8982
Merged
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
Contributor
dlqs
commented
Jul 4, 2021
•
dlqs
commented
- function based hook calls
- almost rewrite of scripting documentation
polychaeta
added
bgp
bugfix
documentation
libfrr
tests
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous 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-20049/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
polychaeta
reviewed
Jul 6, 2021
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/bb708101212924fc35eb787e597097a2/raw/799a9b1bfff3eb17be2a6451104e91ae15dc3798/cr_8982_1625575114.diff | git apply
diff --git a/lib/frrscript.h b/lib/frrscript.h
index 0d5568d34..d03b694b7 100644
--- a/lib/frrscript.h
+++ b/lib/frrscript.h
@@ -187,31 +187,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
* 0 if the script ran successfully, nonzero otherwise.
*/
-#define frrscript_call(fs, f, ...) \
- ({ \
- struct lua_function_state lookup = {.name = f}; \
- struct lua_function_state *lfs; \
- lfs = hash_lookup(fs->lua_function_hash, &lookup); \
- lfs == NULL ? ({ \
- zlog_err( \
- "Lua script call: tried to call '%s' in '%s' which was not loaded", \
- f, fs->name); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
- _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \
- }) != 0 \
- ? ({ \
- zlog_err( \
- "Lua script call: '%s' in '%s' returned non-zero exit code", \
- f, fs->name); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \
- 0; \
- }); \
+#define frrscript_call(fs, f, ...) \
+ ({ \
+ struct lua_function_state lookup = {.name = f}; \
+ struct lua_function_state *lfs; \
+ lfs = hash_lookup(fs->lua_function_hash, &lookup); \
+ lfs == NULL ? ({ \
+ zlog_err( \
+ "Lua script call: tried to call '%s' in '%s' which was not loaded", \
+ f, fs->name); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
+ _frrscript_call_lua( \
+ lfs, PP_NARG(__VA_ARGS__)); \
+ }) != 0 \
+ ? ({ \
+ zlog_err( \
+ "Lua script call: '%s' in '%s' returned non-zero exit code", \
+ f, fs->name); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(DECODE_ARGS, \
+ ##__VA_ARGS__); \
+ 0; \
+ }); \
})
/*
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 3: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO3U18I386-20085/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20085/artifact/TOPO3U18I386/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 i386 part 3: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO3U18I386-20085/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20085/artifact/TOPO3U18I386/ErrorLog/log_topotests.txt |
polychaeta
previously requested changes
Jul 7, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits has a missing or badly formatted
Signed-off-byline; we can't accept your contribution until all of your commits have one - One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/6963e7101fa982398bc6f7e316dcccf6/raw/cf99d1a3d8a33c77aa2399324139b8237fbd6cf7/cr_8982_1625666067.diff | git apply
diff --git a/lib/frrscript.h b/lib/frrscript.h
index d2791a5a8..85e7b15cd 100644
--- a/lib/frrscript.h
+++ b/lib/frrscript.h
@@ -192,31 +192,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
* 0 if the script ran successfully, nonzero otherwise.
*/
-#define frrscript_call(fs, f, ...) \
- ({ \
- struct lua_function_state lookup = {.name = f}; \
- struct lua_function_state *lfs; \
- lfs = hash_lookup(fs->lua_function_hash, &lookup); \
- lfs == NULL ? ({ \
- zlog_err( \
- "Lua script call: tried to call '%s' in '%s' which was not loaded", \
- f, fs->name); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
- _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \
- }) != 0 \
- ? ({ \
- zlog_err( \
- "Lua script call: '%s' in '%s' returned non-zero exit code", \
- f, fs->name); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \
- 0; \
- }); \
+#define frrscript_call(fs, f, ...) \
+ ({ \
+ struct lua_function_state lookup = {.name = f}; \
+ struct lua_function_state *lfs; \
+ lfs = hash_lookup(fs->lua_function_hash, &lookup); \
+ lfs == NULL ? ({ \
+ zlog_err( \
+ "Lua script call: tried to call '%s' in '%s' which was not loaded", \
+ f, fs->name); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
+ _frrscript_call_lua( \
+ lfs, PP_NARG(__VA_ARGS__)); \
+ }) != 0 \
+ ? ({ \
+ zlog_err( \
+ "Lua script call: '%s' in '%s' returned non-zero exit code", \
+ f, fs->name); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(DECODE_ARGS, \
+ ##__VA_ARGS__); \
+ 0; \
+ }); \
})
/*
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
polychaeta
reviewed
Jul 7, 2021
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/5957ed84dcb1664084dd4e55027bc540/raw/cf99d1a3d8a33c77aa2399324139b8237fbd6cf7/cr_8982_1625666426.diff | git apply
diff --git a/lib/frrscript.h b/lib/frrscript.h
index d2791a5a8..85e7b15cd 100644
--- a/lib/frrscript.h
+++ b/lib/frrscript.h
@@ -192,31 +192,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
* 0 if the script ran successfully, nonzero otherwise.
*/
-#define frrscript_call(fs, f, ...) \
- ({ \
- struct lua_function_state lookup = {.name = f}; \
- struct lua_function_state *lfs; \
- lfs = hash_lookup(fs->lua_function_hash, &lookup); \
- lfs == NULL ? ({ \
- zlog_err( \
- "Lua script call: tried to call '%s' in '%s' which was not loaded", \
- f, fs->name); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
- _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \
- }) != 0 \
- ? ({ \
- zlog_err( \
- "Lua script call: '%s' in '%s' returned non-zero exit code", \
- f, fs->name); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \
- 0; \
- }); \
+#define frrscript_call(fs, f, ...) \
+ ({ \
+ struct lua_function_state lookup = {.name = f}; \
+ struct lua_function_state *lfs; \
+ lfs = hash_lookup(fs->lua_function_hash, &lookup); \
+ lfs == NULL ? ({ \
+ zlog_err( \
+ "Lua script call: tried to call '%s' in '%s' which was not loaded", \
+ f, fs->name); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
+ _frrscript_call_lua( \
+ lfs, PP_NARG(__VA_ARGS__)); \
+ }) != 0 \
+ ? ({ \
+ zlog_err( \
+ "Lua script call: '%s' in '%s' returned non-zero exit code", \
+ f, fs->name); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(DECODE_ARGS, \
+ ##__VA_ARGS__); \
+ 0; \
+ }); \
})
/*
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
polychaeta
previously requested changes
Jul 7, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits has a missing or badly formatted
Signed-off-byline; we can't accept your contribution until all of your commits have one - One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedUbuntu 16.04 i386 build: Failed (click for details)Ubuntu 16.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/ErrorLog/ Ubuntu 16.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/config.status/config.status Ubuntu 16.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/config.log/config.log.gz Ubuntu 16.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/frr.xref.xz/frr.xref.xz Ubuntu 16.04 i386 build: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsUbuntu 16.04 i386 build: Failed (click for details)Ubuntu 16.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/ErrorLog/ Ubuntu 16.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/config.status/config.status Ubuntu 16.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/config.log/config.log.gz Ubuntu 16.04 i386 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20106/artifact/U1604I386/frr.xref.xz/frr.xref.xz Ubuntu 16.04 i386 build: No useful log found |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous 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-20105/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
polychaeta
reviewed
Jul 7, 2021
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/8acf51fca83af605c8499735556a1b13/raw/223b294ea88b66295e34a3c7225d569b79ddd340/cr_8982_1625684869.diff | git apply
diff --git a/lib/frrscript.h b/lib/frrscript.h
index 1f234a38a..e8ede01ab 100644
--- a/lib/frrscript.h
+++ b/lib/frrscript.h
@@ -192,31 +192,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
* 0 if the script ran successfully, nonzero otherwise.
*/
-#define frrscript_call(fs, f, ...) \
- ({ \
- struct lua_function_state lookup = {.name = f}; \
- struct lua_function_state *lfs; \
- lfs = hash_lookup(fs->lua_function_hash, &lookup); \
- lfs == NULL ? ({ \
- zlog_err( \
- "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \
- fs->name, f); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
- _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \
- }) != 0 \
- ? ({ \
- zlog_err( \
- "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
- fs->name, f); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \
- 0; \
- }); \
+#define frrscript_call(fs, f, ...) \
+ ({ \
+ struct lua_function_state lookup = {.name = f}; \
+ struct lua_function_state *lfs; \
+ lfs = hash_lookup(fs->lua_function_hash, &lookup); \
+ lfs == NULL ? ({ \
+ zlog_err( \
+ "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \
+ fs->name, f); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
+ _frrscript_call_lua( \
+ lfs, PP_NARG(__VA_ARGS__)); \
+ }) != 0 \
+ ? ({ \
+ zlog_err( \
+ "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
+ fs->name, f); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(DECODE_ARGS, \
+ ##__VA_ARGS__); \
+ 0; \
+ }); \
})
/*
diff --git a/tests/lib/test_frrscript.c b/tests/lib/test_frrscript.c
index b4992f483..e01807f19 100644
--- a/tests/lib/test_frrscript.c
+++ b/tests/lib/test_frrscript.c
@@ -75,7 +75,8 @@ int main(int argc, char **argv)
assert(result == 1);
/* Get result from a function that was not loaded */
- long long *llptr = frrscript_get_result(fs, "does_not_exist", "c", lua_tointegerp);
+ long long *llptr =
+ frrscript_get_result(fs, "does_not_exist", "c", lua_tointegerp);
assert(llptr == NULL);
/* Function returns void */
@@ -89,7 +90,8 @@ int main(int argc, char **argv)
/* Get non-existent result from a function */
result = frrscript_call(fs, "bad_return3");
assert(result == 1);
- long long *cllptr = frrscript_get_result(fs, "bad_return3", "c", lua_tointegerp);
+ long long *cllptr =
+ frrscript_get_result(fs, "bad_return3", "c", lua_tointegerp);
assert(cllptr == NULL);
/* Function throws exception */
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
polychaeta
reviewed
Jul 7, 2021
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/7272de8d8b8f7774ce72fa55691cdd97/raw/bb6d5df6fb506ad9a63c5dcf05638162028e6ea3/cr_8982_1625684894.diff | git apply
diff --git a/lib/frrscript.h b/lib/frrscript.h
index 1f234a38a..e8ede01ab 100644
--- a/lib/frrscript.h
+++ b/lib/frrscript.h
@@ -192,31 +192,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
* 0 if the script ran successfully, nonzero otherwise.
*/
-#define frrscript_call(fs, f, ...) \
- ({ \
- struct lua_function_state lookup = {.name = f}; \
- struct lua_function_state *lfs; \
- lfs = hash_lookup(fs->lua_function_hash, &lookup); \
- lfs == NULL ? ({ \
- zlog_err( \
- "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \
- fs->name, f); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
- _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \
- }) != 0 \
- ? ({ \
- zlog_err( \
- "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
- fs->name, f); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \
- 0; \
- }); \
+#define frrscript_call(fs, f, ...) \
+ ({ \
+ struct lua_function_state lookup = {.name = f}; \
+ struct lua_function_state *lfs; \
+ lfs = hash_lookup(fs->lua_function_hash, &lookup); \
+ lfs == NULL ? ({ \
+ zlog_err( \
+ "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \
+ fs->name, f); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
+ _frrscript_call_lua( \
+ lfs, PP_NARG(__VA_ARGS__)); \
+ }) != 0 \
+ ? ({ \
+ zlog_err( \
+ "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
+ fs->name, f); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(DECODE_ARGS, \
+ ##__VA_ARGS__); \
+ 0; \
+ }); \
})
/*
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 🚧Basic BGPD CI results: Partial FAILURE, 1 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous 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-20107/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
Continuous Integration Result: SUCCESSFULContinuous 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-20110/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6DEB10AMD64-20109/test Topology Tests failed for Topotests debian 10 amd64 part 6: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20109/artifact/TOPO6DEB10AMD64/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests debian 10 amd64 part 6: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO6DEB10AMD64-20109/test Topology Tests failed for Topotests debian 10 amd64 part 6: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20109/artifact/TOPO6DEB10AMD64/ErrorLog/log_topotests.txt |
polychaeta
previously requested changes
Jul 8, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits has a missing or badly formatted
Signed-off-byline; we can't accept your contribution until all of your commits have one - One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/9864e8dc6d7bc55ed16dae26acfa01d7/raw/4cfe3ce082addcf0ea3e0162e931cd1cae946c8d/cr_8982_1625766204.diff | git apply
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 0f4d09664..2422e4f95 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -389,20 +389,20 @@ route_match_script(void *rule, const struct prefix *prefix, void *object)
struct attr newattr = *path->attr;
int result = frrscript_call(
- fs, routematch_function,
- ("prefix", prefix), ("attributes", &newattr), ("peer", path->peer),
+ fs, routematch_function, ("prefix", prefix),
+ ("attributes", &newattr), ("peer", path->peer),
("RM_FAILURE", (long long *)&status_failure),
("RM_NOMATCH", (long long *)&status_nomatch),
("RM_MATCH", (long long *)&status_match),
- ("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change)
- );
+ ("RM_MATCH_AND_CHANGE", (long long *)&status_match_and_change));
if (result) {
zlog_err("Issue running script rule; defaulting to no match");
return RMAP_NOMATCH;
}
- long long* action = frrscript_get_result(fs, routematch_function, "action", lua_tointegerp);
+ long long *action = frrscript_get_result(fs, routematch_function,
+ "action", lua_tointegerp);
int status = RMAP_NOMATCH;
diff --git a/lib/frrscript.h b/lib/frrscript.h
index 14a922a74..3813375ef 100644
--- a/lib/frrscript.h
+++ b/lib/frrscript.h
@@ -192,31 +192,33 @@ int _frrscript_call_lua(struct lua_function_state *lfs, int nargs);
* 0 if the script ran successfully, nonzero otherwise.
*/
-#define frrscript_call(fs, f, ...) \
- ({ \
- struct lua_function_state lookup = {.name = f}; \
- struct lua_function_state *lfs; \
- lfs = hash_lookup(fs->lua_function_hash, &lookup); \
- lfs == NULL ? ({ \
- zlog_err( \
- "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \
- fs->name, f); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
- _frrscript_call_lua(lfs, PP_NARG(__VA_ARGS__)); \
- }) != 0 \
- ? ({ \
- zlog_err( \
- "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
- fs->name, f); \
- 1; \
- }) \
- : ({ \
- MAP_LISTS(DECODE_ARGS, ##__VA_ARGS__); \
- 0; \
- }); \
+#define frrscript_call(fs, f, ...) \
+ ({ \
+ struct lua_function_state lookup = {.name = f}; \
+ struct lua_function_state *lfs; \
+ lfs = hash_lookup(fs->lua_function_hash, &lookup); \
+ lfs == NULL ? ({ \
+ zlog_err( \
+ "frrscript: '%s.lua': '%s': tried to call this function but it was not loaded", \
+ fs->name, f); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(ENCODE_ARGS, ##__VA_ARGS__); \
+ _frrscript_call_lua( \
+ lfs, PP_NARG(__VA_ARGS__)); \
+ }) != 0 \
+ ? ({ \
+ zlog_err( \
+ "frrscript: '%s.lua': '%s': this function called but returned non-zero exit code. No variables modified.", \
+ fs->name, f); \
+ 1; \
+ }) \
+ : ({ \
+ MAP_LISTS(DECODE_ARGS, \
+ ##__VA_ARGS__); \
+ 0; \
+ }); \
})
/*
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 8: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 8: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 8: No useful log found |
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-20487/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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-20488/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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-20489/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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-20490/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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-20495/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
polychaeta
previously requested changes
Jul 26, 2021
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits has a missing or badly formatted
Signed-off-byline; we can't accept your contribution until all of your commits have one - One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Signed-off-by: Donald Lee <dlqs@gmx.com>
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Signed-off-by: Donald Lee <dlqs@gmx.com>
polychaeta
reviewed
Jul 26, 2021
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/6b34a669d0e42a1c63ce904f4d097fe0/raw/ba540a563921494c9587ed15eaf20cda20dd4f30/cr_8982_1627314454.diff | git apply
diff --git a/lib/frrscript.c b/lib/frrscript.c
index c7896f004..3e1e184bc 100644
--- a/lib/frrscript.c
+++ b/lib/frrscript.c
@@ -197,7 +197,7 @@ done:
void *frrscript_get_result(struct frrscript *fs, const char *function_name,
const char *name,
- void * (*lua_to)(lua_State *L, int idx))
+ void *(*lua_to)(lua_State *L, int idx))
{
void *p;
struct lua_function_state *lfs;
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Signed-off-by: Donald Lee <dlqs@gmx.com>
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
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-20538/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
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-20537/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TP1U1804AMD64-20547/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 1: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20547/artifact/TP1U1804AMD64/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 1: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TP1U1804AMD64-20547/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 1: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20547/artifact/TP1U1804AMD64/ErrorLog/log_topotests.txt |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18AMD64-20546/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20546/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO9U18AMD64-20546/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-20546/artifact/TOPO9U18AMD64/ErrorLog/log_topotests.txt |
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-20548/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
qlyoung
reviewed
Jul 28, 2021
lib/frrscript.c
Outdated
|
|
||
| if (snprintf(script_name, sizeof(script_name), "%s/%s.lua", scriptdir, | ||
| fs->name) | ||
| < 0) { |
There was a problem hiding this comment.
This condition is not right. man snprintf:
The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')). If the output was truncated due to this limit, then the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available. Thus, a return value of size or more means that the output was truncated. (See also below under NOTES.)
If an output error is encountered, a negative value is returned.
There was a problem hiding this comment.
updated 9e3a277
I grepped and snprintf <0 isn't usually checked for errors so I just flipped the < to a >=
lib/frrscript.h
Outdated
| #define DECODE_ARGS(name, value) \ | ||
| do { \ | ||
| lua_getglobal(L, name); \ | ||
| DECODE_ARGS_WITH_STATE(L, value); \ | ||
| lua_getfield(lfs->L, 1, name); \ |
There was a problem hiding this comment.
As I mentioned elsewhere you need to wrap macro arguments in (parens) anywhere they are used.
There was a problem hiding this comment.
ah, I thought it was just at that call site. updated b5e790e
added 3 commits
July 29, 2021 05:52
Signed-off-by: Donald Lee <dlqs@gmx.com>
Signed-off-by: Donald Lee <dlqs@gmx.com>
Signed-off-by: Donald Lee <dlqs@gmx.com>
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
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-20613/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings |
qlyoung
approved these changes
Aug 2, 2021
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.