From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/4] mini-os: Fixed shutdown thread Date: Tue, 3 Jun 2014 10:01:27 +0100 Message-ID: <538D8EE7.4040806@citrix.com> References: <1401731397-29842-1-git-send-email-talex5@gmail.com> <1401731397-29842-2-git-send-email-talex5@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wrkb6-0000gl-Qg for xen-devel@lists.xenproject.org; Tue, 03 Jun 2014 09:01:33 +0000 In-Reply-To: <1401731397-29842-2-git-send-email-talex5@gmail.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Thomas Leonard , xen-devel@lists.xenproject.org Cc: samuel.thibault@ens-lyon.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org 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 > --- > 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; > }