[riot-notifications] [RIOT-OS/RIOT] pkg/ubasic: add support for BASIC's "shell" statement. (#11349)

Juan I Carrano notifications at github.com
Mon Apr 8 17:47:58 CEST 2019


jcarrano commented on this pull request.



> @@ -304,3 +304,17 @@ void shell_run(const shell_command_t *shell_commands, char *line_buf, int len)
         print_prompt();
     }
 }
+
+#ifdef MODULE_SHELL_COMMANDS
+/* Provide standard system() function */
+int system(const char *command)
+{
+    int len = strlen(command);
+    char tmpstring[len+1];
+
+    strncpy(tmpstring, command, len+1);
+
+    handle_input_line(_shell_command_list, tmpstring);

I should have used NULL here, since the _shell_command_list is going to be searched anyways.
```suggestion
    handle_input_line(NULL, tmpstring);
```

> @@ -304,3 +304,17 @@ void shell_run(const shell_command_t *shell_commands, char *line_buf, int len)
         print_prompt();
     }
 }
+
+#ifdef MODULE_SHELL_COMMANDS
+/* Provide standard system() function */
+int system(const char *command)

I you don't have shell_commands, then you don't have access to _any_ command from within system(), Because you have no way for your application to communicate a custom command list:

https://github.com/RIOT-OS/RIOT/blob/200c465f36ffee6b1958a59c80b8879d09ea1d2b/sys/shell/shell.c#L54-L62

Nothing will break, but no commands will be available.

In 2038, when we finally get XFAs, this won't be an issue (if we survive the UNIX timestamp overflow)


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11349#pullrequestreview-223928642
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190408/9a9ffe92/attachment.html>


More information about the notifications mailing list