kernel: Change commandline, service name and binary name to const char*

fwsGonzo · fwsGonzo · commit 7bc0bf407e5f · 2017-09-13T06:50:10.000-07:00
diff --git a/api/kernel/os.hpp b/api/kernel/os.hpp
@@ -59,7 +59,7 @@ class OS {
    *  other mechanisms. The first argument is always
    *  the binary name.
   **/
-  static const std::string& cmdline_args() noexcept;
+  static const char* cmdline_args() noexcept;
 
   /** Clock cycles since boot. */
   static uint64_t cycles_since_boot() {
@@ -257,7 +257,6 @@ class OS {
   static bool boot_sequence_passed_;
   static MHz cpu_mhz_;
 
-  // XXX: Only used by solo5
   static RTC::timestamp_t booted_at_;
   static uintptr_t liveupdate_loc_;
   static std::string version_str_;
@@ -266,7 +265,7 @@ class OS {
   static uintptr_t memory_end_;
   static uintptr_t heap_max_;
   static const uintptr_t elf_binary_size_;
-  static std::string cmdline;
+  static const char* cmdline;
 
   // Prohibit copy and move operations
   OS(OS&)  = delete;
diff --git a/api/kernel/service.hpp b/api/kernel/service.hpp
@@ -6,9 +6,9 @@
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -30,12 +30,12 @@ class Service {
   /**
    *  @return: The (descriptive) name of the service
    */
-  static std::string name();
+  static const char* name();
 
   /**
    *  @return: The name of the service binary
    */
-  static std::string binary_name();
+  static const char* binary_name();
 
 
   /**
diff --git a/src/kernel/main_call.cpp b/src/kernel/main_call.cpp
@@ -17,44 +17,3 @@
 
 #include <service>
 #include <info>
-#define ARGS_MAX    64
-
-extern "C" {
-  __attribute__((weak))
-  int main(int, const char*[]);
-}
-
-__attribute__((weak))
-void Service::start(const std::string& cmd)
-{
-  std::string st(cmd); // mangled copy
-  int argc = 0;
-  const char* argv[ARGS_MAX];
-
-  // Get pointers to null-terminated string
-  char* word = (char*) st.c_str();
-  char* end   = word + st.size() + 1;
-  bool new_word = false;
-
-  for (char* ptr = word; ptr < end; ptr++) {
-
-    // Replace all spaces with 0
-    if(std::isspace(*ptr)) {
-      *ptr = 0;
-      new_word = true;
-      continue;
-    }
-
-    // At the start of each word, or last byte, add previous pointer to array
-    if (new_word or ptr == end - 1) {
-      argv[argc++] = word;
-      word = ptr; // next arg
-      if (argc >= ARGS_MAX) break;
-      new_word = false;
-    }
-  }
-
-  int exit_status = main(argc, argv);
-  INFO("main","returned with status %d", exit_status);
-
-}
diff --git a/src/kernel/multiboot.cpp b/src/kernel/multiboot.cpp
@@ -116,7 +116,7 @@ void OS::multiboot(uint32_t boot_addr)
   if (bootinfo_->flags & MULTIBOOT_INFO_CMDLINE) {
     INFO2("* Booted with parameters @ 0x%x: %s", bootinfo_->cmdline,
           reinterpret_cast<const char*>(bootinfo_->cmdline));
-    cmdline = reinterpret_cast<const char*>(bootinfo_->cmdline);
+    OS::cmdline = reinterpret_cast<const char*>(bootinfo_->cmdline);
   }
 
 /*
diff --git a/src/kernel/os.cpp b/src/kernel/os.cpp
@@ -52,7 +52,6 @@ uintptr_t OS::liveupdate_loc_   = 0;
 uintptr_t OS::memory_end_ = 0;
 uintptr_t OS::heap_max_ = (uintptr_t) -1;
 const uintptr_t OS::elf_binary_size_ {(uintptr_t)&_ELF_END_ - (uintptr_t)&_ELF_START_};
-std::string OS::cmdline{Service::binary_name()};
 
 // stdout redirection
 using Print_vec = fixedvector<OS::print_func, 8>;
@@ -73,8 +72,8 @@ void* OS::liveupdate_storage_area() noexcept
   return (void*) OS::liveupdate_loc_;
 }
 
-const std::string& OS::cmdline_args() noexcept
-{
+const char* OS::cmdline = nullptr;
+const char* OS::cmdline_args() noexcept {
   return cmdline;
 }
 
@@ -135,7 +134,7 @@ void OS::post_start()
   printf(" IncludeOS %s (%s / %i-bit)\n",
          version().c_str(), arch().c_str(),
          static_cast<int>(sizeof(uintptr_t)) * 8);
-  printf(" +--> Running [ %s ]\n", Service::name().c_str());
+  printf(" +--> Running [ %s ]\n", Service::name());
   FILLINE('~');
 
   Service::start();
diff --git a/src/kernel/service_stub.cpp b/src/kernel/service_stub.cpp
@@ -17,29 +17,64 @@
 
 #include <os>
 
-// the name of the current service (built from another module)
-extern "C" {
-  __attribute__((weak))
-  const char* service_name__ = "(missing service name)";
-  __attribute__((weak))
-  const char* service_binary_name__ = "(missing binary name)";
-}
+#define ARGS_MAX    64
 
+// name and binary of current service (built from another module)
+extern const char* service_binary_name__;
+extern const char* service_name__;
 
-std::string Service::binary_name() {
+const char* Service::binary_name() {
   return service_binary_name__;
 }
-
-std::string Service::name() {
+const char* Service::name() {
   return service_name__;
 }
 
-
 // functions that we can override if we want to
 __attribute__((weak))
 void Service::start()
 {
-  Service::start(OS::cmdline_args());
+  const std::string args(OS::cmdline_args());
+  Service::start(args);
+}
+
+extern "C" {
+  __attribute__((weak))
+  int main(int, const char*[]) {}
+}
+
+__attribute__((weak))
+void Service::start(const std::string& cmd)
+{
+  std::string st(cmd); // mangled copy
+  int argc = 0;
+  const char* argv[ARGS_MAX];
+
+  // Get pointers to null-terminated string
+  char* word = (char*) st.c_str();
+  char* end  = word + st.size() + 1;
+  bool new_word = false;
+
+  for (char* ptr = word; ptr < end; ptr++) {
+
+    // Replace all spaces with 0
+    if(std::isspace(*ptr)) {
+      *ptr = 0;
+      new_word = true;
+      continue;
+    }
+
+    // At the start of each word, or last byte, add previous pointer to array
+    if (new_word or ptr == end - 1) {
+      argv[argc++] = word;
+      word = ptr; // next arg
+      if (argc >= ARGS_MAX) break;
+      new_word = false;
+    }
+  }
+
+  int exit_status = main(argc, argv);
+  INFO("main","returned with status %d", exit_status);
 }
 
 __attribute__((weak))
diff --git a/src/platform/x86_pc/os.cpp b/src/platform/x86_pc/os.cpp
@@ -96,6 +96,7 @@ void OS::default_stdout(const char* str, const size_t len)
 
 void OS::start(uint32_t boot_magic, uint32_t boot_addr)
 {
+  OS::cmdline = Service::binary_name();
   // Initialize stdout handlers
   OS::add_stdout(&OS::default_stdout);
 
@@ -112,6 +113,10 @@ void OS::start(uint32_t boot_magic, uint32_t boot_addr)
   /// initialize on page 7, 3 pages in size
   Statman::get().init(0x6000, 0x3000);
 
+  // Call global ctors
+  PROFILE("Global constructors");
+  __libc_init_array();
+
   // BOOT METHOD //
   PROFILE("Multiboot / legacy");
   OS::memory_end_ = 0;
@@ -129,10 +134,6 @@ void OS::start(uint32_t boot_magic, uint32_t boot_addr)
   // Give the rest of physical memory to heap
   OS::heap_max_ = OS::memory_end_;
 
-  // Call global ctors
-  PROFILE("Global constructors");
-  __libc_init_array();
-
   PROFILE("Memory map");
   // Assign memory ranges used by the kernel
   auto& memmap = memory_map();
diff --git a/src/platform/x86_solo5/os.cpp b/src/platform/x86_solo5/os.cpp
@@ -86,7 +86,7 @@ void OS::start(char* _cmdline, uintptr_t mem_size)
   /// initialize on page 7, 2 pages in size
   Statman::get().init(0x6000, 0x3000);
 
-  OS::cmdline = reinterpret_cast<char*>(_cmdline);
+  OS::cmdline = _cmdline;
 
   // setup memory and heap end
   OS::memory_end_ = mem_size;
diff --git a/src/plugins/syslogd.cpp b/src/plugins/syslogd.cpp
@@ -102,7 +102,7 @@ void Syslog::syslog(const int priority, const char* buf) {
  	message += std::string{timebuf} + " " + net::Inet4::stack().ip_addr().str() + " ";
 
  	// Fourth: App-name, PROCID (LOG_PID) and MSGID
- 	message += Service::binary_name() + " " + std::to_string(getpid()) + " UDPOUT ";
+ 	message += std::string(Service::binary_name()) + " " + std::to_string(getpid()) + " UDPOUT ";
 
  	/* Structured data: SD-element (SD-ID PARAM-NAME=PARAM-VALUE) */
  	message += "- ";	// NILVALUE
diff --git a/src/util/syslogd.cpp b/src/util/syslogd.cpp
@@ -82,7 +82,7 @@ void Syslog::syslog(const int priority, const char* buf) {
  	message += std::string{timebuf} + " ";
 
  	// Fourth: App-name
- 	message += Service::binary_name();
+ 	message += std::string(Service::binary_name());
 
  	// Fifth: Add ident if is set (through openlog)
  	if (fac_->ident_is_set())
diff --git a/test/kernel/unit/service_stub_test.cpp b/test/kernel/unit/service_stub_test.cpp
@@ -20,10 +20,10 @@
 
 CASE("Service::binary_name() returns name of binary")
 {
-  EXPECT(Service::binary_name() == "(missing binary name)");
+  EXPECT(Service::binary_name() == "Service binary name");
 }
 
 CASE("Service::name() returns name of service")
 {
-  EXPECT(Service::name() == "(missing service name)");
+  EXPECT(Service::name() == "Service name");
 }
diff --git a/test/lest_util/os_mock.cpp b/test/lest_util/os_mock.cpp
@@ -72,6 +72,11 @@ Timers::id_t Timers::periodic(duration_t, duration_t, handler_t) {
   return 0;
 }
 
+#include <service>
+const char* service_binary_name__ = "Service binary name";
+const char* service_name__        = "Service name";
+
+
 #include <kernel/os.hpp>
 void OS::start(unsigned, unsigned) {
 }
diff --git a/test/posix/integration/main/main_no_params.cpp b/test/posix/integration/main/main_no_params.cpp
@@ -1,6 +1,8 @@
-
-#include <stdio.h>
+#include <os>
+#include <cstdio>
 
 int main() {
-  printf("Hello main\n");
+  printf("Hello main - %s\n", OS::cmdline_args());
+  assert(OS::cmdline_args() == std::string("test_main booted with vmrunner"));
+  return 0;
 }
diff --git a/test/posix/integration/main/service.cpp b/test/posix/integration/main/service.cpp
@@ -17,20 +17,19 @@
 
 #include <cstdio>
 #include <string>
-#include <gsl/gsl>
+#include <cassert>
 
-int main(int argc, char** argv) {
-
-  printf("Hello main\n");
+int main(int argc, char** argv)
+{
   printf("Argc: %d\n", argc);
 
   for (int i = 0; i < argc; i++)
     printf("Arg %i: %s\n", i, argv[i]);
 
-  Expects(std::string(argv[0]) == "test_main");
-  Expects(std::string(argv[1]) == "booted");
-  Expects(std::string(argv[2]) == "with");
-  Expects(std::string(argv[3]) == "vmrunner");
+  assert(std::string(argv[0]) == "test_main");
+  assert(std::string(argv[1]) == "booted");
+  assert(std::string(argv[2]) == "with");
+  assert(std::string(argv[3]) == "vmrunner");
 
   // We want to veirify this "exit status" on the back-end
   return 200;

Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,7 @@ void OS::multiboot(uint32_t boot_addr)`
`116`	`116`	`if (bootinfo_->flags & MULTIBOOT_INFO_CMDLINE) {`
`117`	`117`	`INFO2("* Booted with parameters @ 0x%x: %s", bootinfo_->cmdline,`
`118`	`118`	`reinterpret_cast<const char*>(bootinfo_->cmdline));`
`119`		`- cmdline = reinterpret_cast<const char*>(bootinfo_->cmdline);`
	`119`	`+ OS::cmdline = reinterpret_cast<const char*>(bootinfo_->cmdline);`
`120`	`120`	`}`
`121`	`121`
`122`	`122`	`/*`