Re: [PATCH 1/4] mini-os: Fixed shutdown thread

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 02/06/2014 18:49, Thomas Leonard wrote:
> Before, it read "" and started a shutdown immediately. Now, it waits for
> a non-empty value and then actually shuts down.
>
> Signed-off-by: Thomas Leonard <talex5@gmail.com>
> ---
>  extras/mini-os/kernel.c | 16 ++++++++++++----
>  extras/mini-os/main.c   |  2 +-
>  2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/extras/mini-os/kernel.c b/extras/mini-os/kernel.c
> index ea409f4..386be8f 100644
> --- a/extras/mini-os/kernel.c
> +++ b/extras/mini-os/kernel.c
> @@ -69,6 +69,8 @@ void setup_xen_features(void)
>  __attribute__((weak)) void app_shutdown(unsigned reason)
>  {
>      printk("Shutdown requested: %d\n", reason);
> +    struct sched_shutdown sched_shutdown = { .reason = reason };
> +    HYPERVISOR_sched_op(SCHEDOP_shutdown, &sched_shutdown);
>  }
>  
>  static void shutdown_thread(void *p)
> @@ -76,12 +78,18 @@ static void shutdown_thread(void *p)
>      const char *path = "control/shutdown";
>      const char *token = path;
>      xenbus_event_queue events = NULL;
> -    char *shutdown, *err;
> +    char *shutdown = NULL, *err;
>      unsigned int shutdown_reason;
>      xenbus_watch_path_token(XBT_NIL, path, token, &events);
> -    while ((err = xenbus_read(XBT_NIL, path, &shutdown)) != NULL)
> +    while ((err = xenbus_read(XBT_NIL, path, &shutdown)) != NULL || !strcmp(shutdown, ""))
>      {
> -        free(err);
> +        if (err)
> +            free(err);

NULL is a perfectly valid parameter to free(). Please drop the if()

> +        if (shutdown)
> +        {
> +            free(shutdown);
> +            shutdown = NULL;
> +        }

Here as well.

~Andrew

>          xenbus_wait_for_watch(&events);
>      }
>      err = xenbus_unwatch_path_token(XBT_NIL, path, token);
> @@ -106,7 +114,7 @@ static void shutdown_thread(void *p)
>  /* This should be overridden by the application we are linked against. */
>  __attribute__((weak)) int app_main(start_info_t *si)
>  {
> -    printk("Dummy main: start_info=%p\n", si);
> +    printk("kernel.c: dummy main: start_info=%p\n", si);
>      return 0;
>  }
>  
> diff --git a/extras/mini-os/main.c b/extras/mini-os/main.c
> index 73eb6fb..aec0586 100644
> --- a/extras/mini-os/main.c
> +++ b/extras/mini-os/main.c
> @@ -185,7 +185,7 @@ void _exit(int ret)
>  
>  int app_main(start_info_t *si)
>  {
> -    printk("Dummy main: start_info=%p\n", si);
> +    printk("main.c: dummy main: start_info=%p\n", si);
>      main_thread = create_thread("main", call_main, si);
>      return 0;
>  }