Re: [Xen-devel] Mini-os fbfront.c unused variable complaint

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-02-09 at 17:20 +0000, Ian Jackson wrote:
> John McDermott CIV writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable 
> complaint"):
> > There is also an unused variable warning in xenbus/xenbus.c. I think the 
> > compiler should not be warning about this, as the reasonable definition of 
> > the local DEBUG macro is the cause? Anyways, here is an ugly fix that shows 
> > where the problem is raised. I tried the old trick of defining the 
> > variables as volatile, but the new gcc won't let it go by. It won't hurt my 
> > feelings if you don't use this patch.
> 
> Urgh, that "fix" really is very ugly as you say.  I think I'm sold on
> disable the warning.

Please see <1328778306.6133.102.camel@xxxxxxxxxxxxxxxxxxxxxx> for that
patch. But:

In this case:

> @@ -327,13 +331,14 @@ static int allocate_xenbus_id(void)
>  /* Initialise xenbus. */
>  void init_xenbus(void)
>  {
> -    int err;
> +    COND(int err;)
>      printk("Initialising xenbus\n");
>      DEBUG("init_xenbus called.\n");
>      xenstore_buf = mfn_to_virt(start_info.store_mfn);
>      create_thread("xenstore", xenbus_thread_func, NULL);
>      DEBUG("buf at %p.\n", xenstore_buf);
> -    err = bind_evtchn(start_info.store_evtchn,
> +
> +    COND(err = ) bind_evtchn(start_info.store_evtchn,
>                       xenbus_evtchn_handler,
>                NULL);
>      unmask_evtchn(start_info.store_evtchn);

I'd be tempted to suggest that either the DEBUG becomes a normal printk
or is removed. I'd prefer the former since it is a useful init time
message which is only printed once (maybe move the "Initialsing xenbus"
printk to the end "Initialised xenbus on IRQ%d MFN %#lx" or something)

In this case:

> @@ -475,11 +480,17 @@ static void xenbus_debug_msg(const char 
>          { "print", sizeof("print") },
>          { msg, len },
>          { "", 1 }};
> +#ifdef XENBUS_DEBUG
>      struct xsd_sockmsg *reply;
> +#endif
>  
> +#ifdef XENBUS_DEBUG
>      reply = xenbus_msg_reply(XS_DEBUG, 0, req, ARRAY_SIZE(req));
>      DEBUG("Got a reply, type %d, id %d, len %d.\n",
>              reply->type, reply->req_id, reply->len);
> +#else
> +    xenbus_msg_reply(XS_DEBUG, 0, req, ARRAY_SIZE(req));
> +#endif
>  }
>  
>  /* List the contents of a directory.  Returns a malloc()ed array of

I think all the DEBUG's reached from "test_xenbus()" can just become
printks -- the calls from test_xenbus don't serve any other purpose than
to print those messages. This includes the DEBUG message at stake here.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel