Mini-os fbfront.c unused variable complaint

Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

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.

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.

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

Re: Mini-os fbfront.c unused variable complaint

[Keir Fraser]

On 08/02/2012 14:15, "Ian Campbell" <Ian.Campbell@citrix.com> 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).

We should be disablign this warning in Config.mk, where we add
-Wno-unused-but-set-variable to CFLAGS. Perhaps mini-os needs to do the
same, if it is creating its own CFLAGS? Note that we add it in Config.mk via
$(call cc-option-add ...) because older gcc versions do not have this
warning.

 -- Keir

>> 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
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

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

Re: Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

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);
+    }
     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

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

On Wed, 2012-02-08 at 07:19 +0000, Keir Fraser wrote:
> On 08/02/2012 14:15, "Ian Campbell" <Ian.Campbell@citrix.com> 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).
> 
> We should be disablign this warning in Config.mk, where we add
> -Wno-unused-but-set-variable to CFLAGS. Perhaps mini-os needs to do the
> same, if it is creating its own CFLAGS? Note that we add it in Config.mk via
> $(call cc-option-add ...) because older gcc versions do not have this
> warning.

I imagine that would look like the following, John does this work for
you?

On the other hand although this compiler warning is annoying these
particular ones do seem to be real issues worth fixing, if only to
improve the error reporting.

Ian.

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1328778190 0
# Node ID 72db20f4f6389649680e13d8ad6d96b5ed9d576f
# Parent  7f2a3ca9d749c65336aca04617bb0a038445fb49
mini-os: Add -Wno-unused-but-set-variable when compiling mini-oss too.

Disabled for the main Xen build in 23368:0f670f5146c8 but mini-os has it's own
CFLAGS.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 7f2a3ca9d749 -r 72db20f4f638 extras/mini-os/minios.mk
--- a/extras/mini-os/minios.mk	Thu Feb 09 09:03:10 2012 +0000
+++ b/extras/mini-os/minios.mk	Thu Feb 09 09:03:10 2012 +0000
@@ -9,6 +9,7 @@ debug = y
 DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls
 DEF_CFLAGS += $(call cc-option,$(CC),-fno-stack-protector,)
 DEF_CFLAGS += $(call cc-option,$(CC),-fgnu89-inline)
+DEF_CFLAGS += $(call cc-option,$(CC),-Wno-unused-but-set-variable)
 DEF_CFLAGS += -Wstrict-prototypes -Wnested-externs -Wpointer-arith -Winline
 DEF_CPPFLAGS += -D__XEN_INTERFACE_VERSION__=$(XEN_INTERFACE_VERSION)

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

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
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 

Re: Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

Ian,

Here is the first patch. There are other unused variable issues not so easily fixed, so I am putting them in another patch that should come out later today, FWIW.

The changeset id in this patch is bogus. It is based on our own Mercurial repo, not Xenbits. The code is from the RedHat source RPM xen-4.1.2-2.fc16.src.rpm
----


[-- Attachment #2: patch01.diff --]
[-- Type: application/octet-stream, Size: 2311 bytes --]

# HG changeset patch
# Parent 3faf3f1c25fc32a74d248778cc2d9d0a567967cd
mini-os: stop compiler complaint about unused variables

gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1) complains about unused variables
in mini-os drivers

Signed-off-by: John McDermott <john.mcdermott@nrl.navy.mil>

diff -r 3faf3f1c25fc extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c	Mon Jan 09 08:56:52 2012 -0500
+++ b/extras/mini-os/blkfront.c	Thu Feb 09 09:47:34 2012 -0500
@@ -171,6 +171,7 @@ again:
 abort_transaction:
     free(err);
     err = xenbus_transaction_end(xbt, 1, &retry);
+    printk("Abort transaction %s\n", message);
     goto error;
 
 done:
diff -r 3faf3f1c25fc extras/mini-os/console/xencons_ring.c
--- a/extras/mini-os/console/xencons_ring.c	Mon Jan 09 08:56:52 2012 -0500
+++ b/extras/mini-os/console/xencons_ring.c	Thu Feb 09 09:47:34 2012 -0500
@@ -317,6 +317,7 @@ again:
 abort_transaction:
     free(err);
     err = xenbus_transaction_end(xbt, 1, &retry);
+    printk("Abort transaction %s\n", message);
     goto error;
 
 done:
diff -r 3faf3f1c25fc extras/mini-os/fbfront.c
--- a/extras/mini-os/fbfront.c	Mon Jan 09 08:56:52 2012 -0500
+++ b/extras/mini-os/fbfront.c	Thu Feb 09 09:47:34 2012 -0500
@@ -142,6 +142,7 @@ again:
 abort_transaction:
     free(err);
     err = xenbus_transaction_end(xbt, 1, &retry);
+    printk("Abort transaction %s\n", message);
     goto error;
 
 done:
@@ -503,6 +504,7 @@ again:
 abort_transaction:
     free(err);
     err = xenbus_transaction_end(xbt, 1, &retry);
+    printk("Abort transaction %s\n", message);
     goto error;
 
 done:
diff -r 3faf3f1c25fc extras/mini-os/netfront.c
--- a/extras/mini-os/netfront.c	Mon Jan 09 08:56:52 2012 -0500
+++ b/extras/mini-os/netfront.c	Thu Feb 09 09:47:34 2012 -0500
@@ -421,6 +421,7 @@ again:
 abort_transaction:
     free(err);
     err = xenbus_transaction_end(xbt, 1, &retry);
+    printk("Abort transaction %s\n", message);
     goto error;
 
 done:
diff -r 3faf3f1c25fc extras/mini-os/pcifront.c
--- a/extras/mini-os/pcifront.c	Mon Jan 09 08:56:52 2012 -0500
+++ b/extras/mini-os/pcifront.c	Thu Feb 09 09:47:34 2012 -0500
@@ -222,6 +222,7 @@ again:
 abort_transaction:
     free(err);
     err = xenbus_transaction_end(xbt, 1, &retry);
+    printk("Abort transaction %s\n", message);
     goto error;
 
 done:

[-- Attachment #3: Type: text/plain, Size: 628 bytes --]



----
Sincerely,

John

On Feb 9, 2012, at 4:10 AM, Ian Campbell wrote:

> 
> 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.

----
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











[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

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

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

On Thu, 2012-02-09 at 15:28 +0000, John McDermott CIV wrote:
> mini-os: stop compiler complaint about unused variables
> 
> gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1) complains about unused
> variables
> in mini-os drivers
> 
> Signed-off-by: John McDermott <john.mcdermott@nrl.navy.mil>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Although perhaps "$FOO abort transaction", for $FOO in blkfront,netfront
etc might be more helpful if the message actually happens? 

> 
> diff -r 3faf3f1c25fc extras/mini-os/blkfront.c
> --- a/extras/mini-os/blkfront.c Mon Jan 09 08:56:52 2012 -0500
> +++ b/extras/mini-os/blkfront.c Thu Feb 09 09:47:34 2012 -0500
> @@ -171,6 +171,7 @@ again:
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    printk("Abort transaction %s\n", message);
>      goto error;
>  
>  done:
> diff -r 3faf3f1c25fc extras/mini-os/console/xencons_ring.c
> --- a/extras/mini-os/console/xencons_ring.c     Mon Jan 09 08:56:52
> 2012 -0500
> +++ b/extras/mini-os/console/xencons_ring.c     Thu Feb 09 09:47:34
> 2012 -0500
> @@ -317,6 +317,7 @@ again:
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    printk("Abort transaction %s\n", message);
>      goto error;
>  
>  done:
> diff -r 3faf3f1c25fc extras/mini-os/fbfront.c
> --- a/extras/mini-os/fbfront.c  Mon Jan 09 08:56:52 2012 -0500
> +++ b/extras/mini-os/fbfront.c  Thu Feb 09 09:47:34 2012 -0500
> @@ -142,6 +142,7 @@ again:
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    printk("Abort transaction %s\n", message);
>      goto error;
>  
>  done:
> @@ -503,6 +504,7 @@ again:
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    printk("Abort transaction %s\n", message);
>      goto error;
>  
>  done:
> diff -r 3faf3f1c25fc extras/mini-os/netfront.c
> --- a/extras/mini-os/netfront.c Mon Jan 09 08:56:52 2012 -0500
> +++ b/extras/mini-os/netfront.c Thu Feb 09 09:47:34 2012 -0500
> @@ -421,6 +421,7 @@ again:
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    printk("Abort transaction %s\n", message);
>      goto error;
>  
>  done:
> diff -r 3faf3f1c25fc extras/mini-os/pcifront.c
> --- a/extras/mini-os/pcifront.c Mon Jan 09 08:56:52 2012 -0500
> +++ b/extras/mini-os/pcifront.c Thu Feb 09 09:47:34 2012 -0500
> @@ -222,6 +222,7 @@ again:
>  abort_transaction:
>      free(err);
>      err = xenbus_transaction_end(xbt, 1, &retry);
> +    printk("Abort transaction %s\n", message);
>      goto error;
>  
>  done:
> 
> 

Re: Mini-os fbfront.c unused variable complaint

[Ian Jackson]

John McDermott CIV writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable complaint"):
> Here is the first patch. There are other unused variable issues not so easily fixed, so I am putting them in another patch that should come out later today, FWIW.

Great, thanks.

Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

> The changeset id in this patch is bogus. It is based on our own Mercurial repo, not Xenbits. The code is from the RedHat source RPM xen-4.1.2-2.fc16.src.rpm

Noted, ta.

Ian.

Re: Mini-os fbfront.c unused variable complaint

[Ian Jackson]

Keir Fraser writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable complaint"):
> We should be disablign this warning in Config.mk, where we add
> -Wno-unused-but-set-variable to CFLAGS. Perhaps mini-os needs to do the
> same, if it is creating its own CFLAGS? Note that we add it in Config.mk via
> $(call cc-option-add ...) because older gcc versions do not have this
> warning.

Personally in this case I like the new warning.  Shall we at least
wait with disabling it until we get an actual false positive ?

Ian.

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

On Thu, 2012-02-09 at 16:08 +0000, Ian Jackson wrote:
> Keir Fraser writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable complaint"):
> > We should be disablign this warning in Config.mk, where we add
> > -Wno-unused-but-set-variable to CFLAGS. Perhaps mini-os needs to do the
> > same, if it is creating its own CFLAGS? Note that we add it in Config.mk via
> > $(call cc-option-add ...) because older gcc versions do not have this
> > warning.
> 
> Personally in this case I like the new warning.  Shall we at least
> wait with disabling it until we get an actual false positive ?

Yes, lets.

Ian

Re: Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

Ian,

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.



[-- Attachment #2: patch02.diff --]
[-- Type: application/octet-stream, Size: 1975 bytes --]

# HG changeset patch
# Parent 4b43dc4c31c3daa6ca2544af062044210f7ad189
mini-os: stop compiler complaint about unused variables

gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1) complains about unused variables
in mini-os drivers. In this case, gcc is probably wrong to warn about
the construction of the DEBUG macro.

Signed-off-by: John McDermott <john.mcdermott@nrl.navy.mil>

diff -r 4b43dc4c31c3 extras/mini-os/xenbus/xenbus.c
--- a/extras/mini-os/xenbus/xenbus.c	Thu Feb 09 09:47:34 2012 -0500
+++ b/extras/mini-os/xenbus/xenbus.c	Thu Feb 09 11:58:10 2012 -0500
@@ -37,8 +37,10 @@
 #ifdef XENBUS_DEBUG
 #define DEBUG(_f, _a...) \
     printk("MINI_OS(file=xenbus.c, line=%d) " _f , __LINE__, ## _a)
+#define COND(e) e
 #else
 #define DEBUG(_f, _a...)    ((void)0)
+#define COND(e)
 #endif
 
 static struct xenstore_domain_interface *xenstore_buf;
@@ -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);
@@ -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

[-- Attachment #3: Type: text/plain, Size: 985 bytes --]



Sincerely,

John
 
On Feb 9, 2012, at 11:09 AM, Ian Campbell wrote:

> On Thu, 2012-02-09 at 16:08 +0000, Ian Jackson wrote:
>> Keir Fraser writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable complaint"):
>>> We should be disablign this warning in Config.mk, where we add
>>> -Wno-unused-but-set-variable to CFLAGS. Perhaps mini-os needs to do the
>>> same, if it is creating its own CFLAGS? Note that we add it in Config.mk via
>>> $(call cc-option-add ...) because older gcc versions do not have this
>>> warning.
>> 
>> Personally in this case I like the new warning.  Shall we at least
>> wait with disabling it until we get an actual false positive ?
> 
> Yes, lets.
> 
> Ian

----
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











[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

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

Re: Mini-os fbfront.c unused variable complaint

[Ian Jackson]

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.

Ian.

Re: Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

Ian,

I would expect most use of mini-os would involve lots of locally hacking the source anyways (we do) and likely to raise this warning again. So it is probably the best choice to disable?

Sincerely,

John
----
On Feb 9, 2012, at 12:20 PM, 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.
> 
> Ian.

----
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

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

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@zakaz.uk.xensource.com> 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.

Re: Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

Ian,

So like this? I took out the linefeed between the 2 printk in init_xenbus, to save a line of terminal output. (Mercurial foo is our internal number.)

Sincerely,

John
----

# HG changeset patch
# Parent 4b43dc4c31c3daa6ca2544af062044210f7ad189
mini-os: stop compiler complaint about unused variables

gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1) complains about unused variables
in mini-os drivers

Signed-off-by: John McDermott <john.mcdermott@nrl.navy.mil>

diff -r 4b43dc4c31c3 extras/mini-os/xenbus/xenbus.c
--- a/extras/mini-os/xenbus/xenbus.c	Thu Feb 09 09:47:34 2012 -0500
+++ b/extras/mini-os/xenbus/xenbus.c	Mon Feb 13 09:50:12 2012 -0500
@@ -328,16 +328,16 @@ static int allocate_xenbus_id(void)
 void init_xenbus(void)
 {
     int err;
-    printk("Initialising xenbus\n");
-    DEBUG("init_xenbus called.\n");
+    printk("Initialising xenbus. ");
+    printk("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);
+    printk("buf at %p.\n", xenstore_buf);
     err = bind_evtchn(start_info.store_evtchn,
 		      xenbus_evtchn_handler,
               NULL);
     unmask_evtchn(start_info.store_evtchn);
-    DEBUG("xenbus on irq %d\n", err);
+    printk("xenbus on irq %d\n", err);
 }
 
 void fini_xenbus(void)
@@ -478,7 +478,7 @@ static void xenbus_debug_msg(const char 
     struct xsd_sockmsg *reply;
 
     reply = xenbus_msg_reply(XS_DEBUG, 0, req, ARRAY_SIZE(req));
-    DEBUG("Got a reply, type %d, id %d, len %d.\n",
+    printk("Got a reply, type %d, id %d, len %d.\n",
             reply->type, reply->req_id, reply->len);
 }
 


On Feb 13, 2012, at 7:43 AM, Ian Campbell wrote:

> 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.

----
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

Re: Mini-os fbfront.c unused variable complaint

[Ian Campbell]

On Mon, 2012-02-13 at 14:53 +0000, John McDermott CIV wrote:
> Ian,
> 
> So like this?
I was thinking more like the following, e..g change all the test_* DEBUG
into print (for consistency) and make the init_xenbus messages more
useful while also using the unused var.

Only tested with my compiler which does not complain in the same way as
yours.

Ian.

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1329145492 0
# Node ID 858955a31cf51baef86c885af3db7fc85a04ec17
# Parent  4ed7fb2c7bd837a33ef7abbd5fbef6cc4c9992df
minios: Remove unused variables warnings

s/DEBUG/printk/ in test_xenbus and all associated do_*_test+xenbus_dbg_message
and always print the IRQ and MFN used by the xenbus on init.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 4ed7fb2c7bd8 -r 858955a31cf5 extras/mini-os/xenbus/xenbus.c
--- a/extras/mini-os/xenbus/xenbus.c	Mon Feb 13 13:00:36 2012 +0000
+++ b/extras/mini-os/xenbus/xenbus.c	Mon Feb 13 15:04:52 2012 +0000
@@ -328,7 +328,6 @@ static int allocate_xenbus_id(void)
 void init_xenbus(void)
 {
     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);
@@ -337,7 +336,8 @@ void init_xenbus(void)
 		      xenbus_evtchn_handler,
               NULL);
     unmask_evtchn(start_info.store_evtchn);
-    DEBUG("xenbus on irq %d\n", err);
+    printk("xenbus initialised on irq %d mfn %#lx\n",
+	   err, start_info.store_mfn);
 }
 
 void fini_xenbus(void)
@@ -478,7 +478,7 @@ static void xenbus_debug_msg(const char 
     struct xsd_sockmsg *reply;
 
     reply = xenbus_msg_reply(XS_DEBUG, 0, req, ARRAY_SIZE(req));
-    DEBUG("Got a reply, type %d, id %d, len %d.\n",
+    printk("Got a reply, type %d, id %d, len %d.\n",
             reply->type, reply->req_id, reply->len);
 }
 
@@ -752,16 +752,16 @@ static void do_ls_test(const char *pre)
     char **dirs, *msg;
     int x;
 
-    DEBUG("ls %s...\n", pre);
+    printk("ls %s...\n", pre);
     msg = xenbus_ls(XBT_NIL, pre, &dirs);
     if (msg) {
-	DEBUG("Error in xenbus ls: %s\n", msg);
+	printk("Error in xenbus ls: %s\n", msg);
 	free(msg);
 	return;
     }
     for (x = 0; dirs[x]; x++) 
     {
-        DEBUG("ls %s[%d] -> %s\n", pre, x, dirs[x]);
+        printk("ls %s[%d] -> %s\n", pre, x, dirs[x]);
         free(dirs[x]);
     }
     free(dirs);
@@ -770,68 +770,68 @@ static void do_ls_test(const char *pre)
 static void do_read_test(const char *path)
 {
     char *res, *msg;
-    DEBUG("Read %s...\n", path);
+    printk("Read %s...\n", path);
     msg = xenbus_read(XBT_NIL, path, &res);
     if (msg) {
-	DEBUG("Error in xenbus read: %s\n", msg);
+	printk("Error in xenbus read: %s\n", msg);
 	free(msg);
 	return;
     }
-    DEBUG("Read %s -> %s.\n", path, res);
+    printk("Read %s -> %s.\n", path, res);
     free(res);
 }
 
 static void do_write_test(const char *path, const char *val)
 {
     char *msg;
-    DEBUG("Write %s to %s...\n", val, path);
+    printk("Write %s to %s...\n", val, path);
     msg = xenbus_write(XBT_NIL, path, val);
     if (msg) {
-	DEBUG("Result %s\n", msg);
+	printk("Result %s\n", msg);
 	free(msg);
     } else {
-	DEBUG("Success.\n");
+	printk("Success.\n");
     }
 }
 
 static void do_rm_test(const char *path)
 {
     char *msg;
-    DEBUG("rm %s...\n", path);
+    printk("rm %s...\n", path);
     msg = xenbus_rm(XBT_NIL, path);
     if (msg) {
-	DEBUG("Result %s\n", msg);
+	printk("Result %s\n", msg);
 	free(msg);
     } else {
-	DEBUG("Success.\n");
+	printk("Success.\n");
     }
 }
 
 /* Simple testing thing */
 void test_xenbus(void)
 {
-    DEBUG("Doing xenbus test.\n");
+    printk("Doing xenbus test.\n");
     xenbus_debug_msg("Testing xenbus...\n");
 
-    DEBUG("Doing ls test.\n");
+    printk("Doing ls test.\n");
     do_ls_test("device");
     do_ls_test("device/vif");
     do_ls_test("device/vif/0");
 
-    DEBUG("Doing read test.\n");
+    printk("Doing read test.\n");
     do_read_test("device/vif/0/mac");
     do_read_test("device/vif/0/backend");
 
-    DEBUG("Doing write test.\n");
+    printk("Doing write test.\n");
     do_write_test("device/vif/0/flibble", "flobble");
     do_read_test("device/vif/0/flibble");
     do_write_test("device/vif/0/flibble", "widget");
     do_read_test("device/vif/0/flibble");
 
-    DEBUG("Doing rm test.\n");
+    printk("Doing rm test.\n");
     do_rm_test("device/vif/0/flibble");
     do_read_test("device/vif/0/flibble");
-    DEBUG("(Should have said ENOENT)\n");
+    printk("(Should have said ENOENT)\n");
 }
 
 /*

Re: Mini-os fbfront.c unused variable complaint

[John McDermott CIV]

Ian,

My compiler (Fedora 16) is happy with that.


Sincerely,

John
----
On Feb 13, 2012, at 10:06 AM, Ian Campbell wrote:

> On Mon, 2012-02-13 at 14:53 +0000, John McDermott CIV wrote:
>> Ian,
>> 
>> So like this?
> I was thinking more like the following, e..g change all the test_* DEBUG
> into print (for consistency) and make the init_xenbus messages more
> useful while also using the unused var.
> 
> Only tested with my compiler which does not complain in the same way as
> yours.
> 
> Ian.
> 
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # Date 1329145492 0
> # Node ID 858955a31cf51baef86c885af3db7fc85a04ec17
> # Parent  4ed7fb2c7bd837a33ef7abbd5fbef6cc4c9992df
> minios: Remove unused variables warnings
> 
> s/DEBUG/printk/ in test_xenbus and all associated do_*_test+xenbus_dbg_message
> and always print the IRQ and MFN used by the xenbus on init.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> 
> diff -r 4ed7fb2c7bd8 -r 858955a31cf5 extras/mini-os/xenbus/xenbus.c
> --- a/extras/mini-os/xenbus/xenbus.c	Mon Feb 13 13:00:36 2012 +0000
> +++ b/extras/mini-os/xenbus/xenbus.c	Mon Feb 13 15:04:52 2012 +0000
> @@ -328,7 +328,6 @@ static int allocate_xenbus_id(void)
> void init_xenbus(void)
> {
>     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);
> @@ -337,7 +336,8 @@ void init_xenbus(void)
> 		      xenbus_evtchn_handler,
>               NULL);
>     unmask_evtchn(start_info.store_evtchn);
> -    DEBUG("xenbus on irq %d\n", err);
> +    printk("xenbus initialised on irq %d mfn %#lx\n",
> +	   err, start_info.store_mfn);
> }
> 
> void fini_xenbus(void)
> @@ -478,7 +478,7 @@ static void xenbus_debug_msg(const char 
>     struct xsd_sockmsg *reply;
> 
>     reply = xenbus_msg_reply(XS_DEBUG, 0, req, ARRAY_SIZE(req));
> -    DEBUG("Got a reply, type %d, id %d, len %d.\n",
> +    printk("Got a reply, type %d, id %d, len %d.\n",
>             reply->type, reply->req_id, reply->len);
> }
> 
> @@ -752,16 +752,16 @@ static void do_ls_test(const char *pre)
>     char **dirs, *msg;
>     int x;
> 
> -    DEBUG("ls %s...\n", pre);
> +    printk("ls %s...\n", pre);
>     msg = xenbus_ls(XBT_NIL, pre, &dirs);
>     if (msg) {
> -	DEBUG("Error in xenbus ls: %s\n", msg);
> +	printk("Error in xenbus ls: %s\n", msg);
> 	free(msg);
> 	return;
>     }
>     for (x = 0; dirs[x]; x++) 
>     {
> -        DEBUG("ls %s[%d] -> %s\n", pre, x, dirs[x]);
> +        printk("ls %s[%d] -> %s\n", pre, x, dirs[x]);
>         free(dirs[x]);
>     }
>     free(dirs);
> @@ -770,68 +770,68 @@ static void do_ls_test(const char *pre)
> static void do_read_test(const char *path)
> {
>     char *res, *msg;
> -    DEBUG("Read %s...\n", path);
> +    printk("Read %s...\n", path);
>     msg = xenbus_read(XBT_NIL, path, &res);
>     if (msg) {
> -	DEBUG("Error in xenbus read: %s\n", msg);
> +	printk("Error in xenbus read: %s\n", msg);
> 	free(msg);
> 	return;
>     }
> -    DEBUG("Read %s -> %s.\n", path, res);
> +    printk("Read %s -> %s.\n", path, res);
>     free(res);
> }
> 
> static void do_write_test(const char *path, const char *val)
> {
>     char *msg;
> -    DEBUG("Write %s to %s...\n", val, path);
> +    printk("Write %s to %s...\n", val, path);
>     msg = xenbus_write(XBT_NIL, path, val);
>     if (msg) {
> -	DEBUG("Result %s\n", msg);
> +	printk("Result %s\n", msg);
> 	free(msg);
>     } else {
> -	DEBUG("Success.\n");
> +	printk("Success.\n");
>     }
> }
> 
> static void do_rm_test(const char *path)
> {
>     char *msg;
> -    DEBUG("rm %s...\n", path);
> +    printk("rm %s...\n", path);
>     msg = xenbus_rm(XBT_NIL, path);
>     if (msg) {
> -	DEBUG("Result %s\n", msg);
> +	printk("Result %s\n", msg);
> 	free(msg);
>     } else {
> -	DEBUG("Success.\n");
> +	printk("Success.\n");
>     }
> }
> 
> /* Simple testing thing */
> void test_xenbus(void)
> {
> -    DEBUG("Doing xenbus test.\n");
> +    printk("Doing xenbus test.\n");
>     xenbus_debug_msg("Testing xenbus...\n");
> 
> -    DEBUG("Doing ls test.\n");
> +    printk("Doing ls test.\n");
>     do_ls_test("device");
>     do_ls_test("device/vif");
>     do_ls_test("device/vif/0");
> 
> -    DEBUG("Doing read test.\n");
> +    printk("Doing read test.\n");
>     do_read_test("device/vif/0/mac");
>     do_read_test("device/vif/0/backend");
> 
> -    DEBUG("Doing write test.\n");
> +    printk("Doing write test.\n");
>     do_write_test("device/vif/0/flibble", "flobble");
>     do_read_test("device/vif/0/flibble");
>     do_write_test("device/vif/0/flibble", "widget");
>     do_read_test("device/vif/0/flibble");
> 
> -    DEBUG("Doing rm test.\n");
> +    printk("Doing rm test.\n");
>     do_rm_test("device/vif/0/flibble");
>     do_read_test("device/vif/0/flibble");
> -    DEBUG("(Should have said ENOENT)\n");
> +    printk("(Should have said ENOENT)\n");
> }
> 
> /*
> 

----
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

Re: Mini-os fbfront.c unused variable complaint

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable complaint"):
> s/DEBUG/printk/ in test_xenbus and all associated do_*_test+xenbus_dbg_message
> and always print the IRQ and MFN used by the xenbus on init.

This looks plausible to me.  Anyone else have any comments ?

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

Re: Mini-os fbfront.c unused variable complaint

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] Mini-os fbfront.c unused variable complaint"):
> minios: Remove unused variables warnings
> 
> s/DEBUG/printk/ in test_xenbus and all associated do_*_test+xenbus_dbg_message
> and always print the IRQ and MFN used by the xenbus on init.

Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>