Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2012-02-08 at 15:01 +0000, John McDermott CIV wrote:
> Ian,
> 
> Agreed; this warning is very annoying. The problem is in several
> files, like a design pattern issue. So far it has popped up in
> fbfront.c, blkfront.c, and netfront.c, but the code is nice clean
> code, so the problem is very regular. We are running Fedora 16 and Xen
> 4.1.2.
> 
> ----
> 
> [mc@xenpro3 ~]$ gcc --version
> gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)
> Copyright (C) 2011 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> ----
> 
> Here is an example of what I have been doing to get rid of the warning:
> 
> ----
> [mc@xenpro3 xenon]$ hg diff extras/mini-os/fbfront.c
> diff -r 8cfe801d065d extras/mini-os/fbfront.c
> --- a/extras/mini-os/fbfront.c	Mon Feb 06 08:37:59 2012 -0500
> +++ b/extras/mini-os/fbfront.c	Wed Feb 08 09:52:06 2012 -0500
> @@ -142,6 +142,9 @@
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    if (message) {
> +	printk("Abort transaction %s", message);
> +    }

I'd be tempted to make these unconditional and then fixup any resultant
"variable message is used but not initialised" type warnings since I
expect we should always have a reason for getting to this particular
point so it would be a bug not to set message at that time.

Would you mind resending this with a Signed-off-by as per
http://wiki.xen.org/wiki/SubmittingXenPatches ? From what you say above
I guess you also have similar fixes to the other drivers, I think it
would be fine to bundle all those similar fixes into a single patch.

Thanks, 
Ian.
>      goto error;
>  
>  done:
> @@ -503,6 +506,9 @@
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    if (message) {
> +	printk("Abort transaction %s", message);
> +    }
>      goto error;
>  
> 
>  done:
> [mc@xenpro3 xenon]$
> ----
> 
> On Feb 8, 2012, at 9:15 AM, Ian Campbell wrote:
> 
> > On Mon, 2012-02-06 at 19:05 +0000, John McDermott CIV wrote:
> >> Xen Developers,
> >> 
> >> FWIW, in trying to compile mini-os on Xen 4.1.2, I get a variable set
> >> but not used warning for variable 'message' in function init_kbdfront.
> >> (Looks like another one just like it, in function init_fbfront, from
> >> the same file.) My source won't compile with these warnings. I could
> >> not find a patch for this in xen-4.1-testing.hg Xenbits.
> > 
> > This new, enabled by default, compiler warning is very annoying, even
> > though it happens to be correct. Which distro has done it? (or maybe
> > it's just new enough gcc which is to blame).
> > 
> >> It looks like a conditional printk got left out after the label
> >> 'abort_transaction'. I don't understand fbfront.c well enough to
> >> suggest a patch. Its low priority for me, because I am going to stick
> >> some plausible code in there for now.
> > 
> > I think an unconditional printk including the %s and message is all
> > which is required here. If you've got some plausible code you may as
> > well post it.
> > 
> > Thanks,
> > Ian.
> > 
> >> 
> >> Sincerely,
> >> 
> >> John
> >> ----
> >> What is the formal meaning of the one-line program
> >> #include "/dev/tty"
> >> 
> >> J.P. McDermott			building 12
> >> Code 5542			john.mcdermott@nrl.navy.mil
> >> Naval Research Laboratory	voice: +1 202.404.8301
> >> Washington, DC 20375, US	fax:   +1 202.404.7942
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> 
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> > 
> 
> ----
> What is the formal meaning of the one-line program
> #include "/dev/tty"
> 
> J.P. McDermott			building 12
> Code 5542			john.mcdermott@nrl.navy.mil
> Naval Research Laboratory	voice: +1 202.404.8301
> Washington, DC 20375, US	fax:   +1 202.404.7942
> 
> 
> 
> 
> 
> 
> 
> 
> 
>