Skip to content

sys/shell/commands: add static qualifier where appropriate#18179

Merged
benpicco merged 3 commits intoRIOT-OS:masterfrom
maribu:sys/shell_commands
Jun 14, 2022
Merged

sys/shell/commands: add static qualifier where appropriate#18179
benpicco merged 3 commits intoRIOT-OS:masterfrom
maribu:sys/shell_commands

Conversation

@maribu
Copy link
Member

@maribu maribu commented Jun 8, 2022

Contribution description

Due to the conversion to XFA based SHELL_COMMAND() much fewer function need to expose a symbol. Hence, spray static all over the place.

Testing procedure

One could search for missing static using this regex:

$ rg '^[ \t]*int[ \t]*[a-zA-Z0-9_]*[ \t]*\([ \t]*int[ \t]*[a-zA-Z0-9_]*[ \t]*,[ \t]*char[ \t*]*[a-zA-Z0-9_]*[ \t]*\)'

Murdock will check that I didn't add a static where it shouldn't be.

Issues/PRs references

Follow up to #18152

@github-actions github-actions bot added the Area: sys Area: System label Jun 8, 2022
@maribu maribu added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 8, 2022
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

@kaspar030 kaspar030 enabled auto-merge June 8, 2022 08:31
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 8, 2022
@maribu maribu force-pushed the sys/shell_commands branch from d7cabbc to e7c58a5 Compare June 10, 2022 06:46
@maribu
Copy link
Member Author

maribu commented Jun 10, 2022

I had to remove the static again from _gnrc_neitf_config() and _gnrc_ipv6_nib() to get tests/gnrc_dhcpv6_client compiling again.

@miri64: I think that something like this would allow tests/gnrc_dhcpv6_client to work without using shell commands:

diff --git a/tests/gnrc_dhcpv6_client/main.c b/tests/gnrc_dhcpv6_client/main.c
index 19741e5752..f51ae66328 100644
--- a/tests/gnrc_dhcpv6_client/main.c
+++ b/tests/gnrc_dhcpv6_client/main.c
@@ -20,13 +20,13 @@
 #include <stddef.h>
 
 #include "net/gnrc/netif.h"
+#include "net/gnrc/ipv6/nib.h"
 #include "net/dhcpv6/client.h"
 #include "net/sock.h"
 #include "xtimer.h"
 
 static char _dhcpv6_client_stack[DHCPV6_CLIENT_STACK_SIZE];
 
-extern int _gnrc_netif_config(int argc, char **argv);
 extern int _gnrc_ipv6_nib(int argc, char **argv);
 
 void *_dhcpv6_client_thread(void *args)
@@ -55,15 +55,25 @@ void *_dhcpv6_client_thread(void *args)
 
 int main(void)
 {
-    char *pl[] = { "nib", "prefix" };
-
-    _gnrc_netif_config(0, NULL);
+    /* print all IPv6 addresses */
+    printf("At start: {\"IPv6 addresses\": [\"");
+    netifs_print_ipv6("\", \"");
+    puts("\"]}");
     thread_create(_dhcpv6_client_stack, DHCPV6_CLIENT_STACK_SIZE,
                   DHCPV6_CLIENT_PRIORITY, THREAD_CREATE_STACKTEST,
                   _dhcpv6_client_thread, NULL, "dhcpv6-client");
     xtimer_sleep(5);
     /* global address should now be configured */
-    _gnrc_netif_config(0, NULL);
-    _gnrc_ipv6_nib(2, pl);
+    printf("After 5 seconds: {\"IPv6 addresses\": [\"");
+    netifs_print_ipv6("\", \"");
+    puts("\"]}");
+    gnrc_ipv6_nib_nc_t entry;
+    void *state = NULL;
+    unsigned iface = 0U;
+
+    while (gnrc_ipv6_nib_nc_iter(iface, &state, &entry)) {
+        gnrc_ipv6_nib_nc_print(&entry);
+    }
+
     return 0;
 }

But the python test script needs to be adapted as well. The test script doesn't run on my machine, so I would rather leave this to someone else.

Rather than abusing _ps_handler() to call ps(), just call it directly.
@maribu maribu force-pushed the sys/shell_commands branch from e7c58a5 to aed2117 Compare June 10, 2022 09:14
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Jun 10, 2022
maribu added 2 commits June 11, 2022 14:38
To allow marking the shell command static, add a trivial
_show_blockers() function that lists the current pm layered blocker
state.
Due to the conversion to XFA based SHELL_COMMAND() much fewer function
need to expose a symbol. Hence, spray `static` all over the place.
@maribu maribu force-pushed the sys/shell_commands branch from aed2117 to 4f769c2 Compare June 11, 2022 12:39
@maribu maribu disabled auto-merge June 11, 2022 19:00
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 11, 2022
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 12, 2022
@maribu
Copy link
Member Author

maribu commented Jun 13, 2022

All green. 9b0cf39 got added post ACK

@benpicco benpicco enabled auto-merge June 14, 2022 08:05
@benpicco benpicco merged commit 6a15ad4 into RIOT-OS:master Jun 14, 2022
@maribu maribu deleted the sys/shell_commands branch July 22, 2022 16:16
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants