On Thu, Nov 10, 2011 at 11:11:18AM -0500, Alan Stern wrote:
> On Thu, 10 Nov 2011, Felipe Balbi wrote:
>
> > On Thu, Nov 10, 2011 at 10:47:12AM +0200, Felipe Balbi wrote:
> > > Fix the following compile warning:
> > >
> > > | In file included from drivers/usb/host/ohci-hcd.c:101:0:
> > > | drivers/usb/host/ohci-dbg.c: In function �@^Xfill_registers_buffer�@^Y:
> > > | drivers/usb/host/ohci-dbg.c:642:2: warning: the comparison will \
> > > | always evaluate as �@^Xtrue�@^Y for the address of �@^Xnext�@^Y wil$
> > > | never be NULL [-Waddress]
> > > | drivers/usb/host/ohci-dbg.c:661:3: warning: the comparison will \
> > > | always evaluate as �@^Xtrue�@^Y for the address of �@^Xnext�@^Y wil$
> > > | never be NULL [-Waddress]
>
> I don't know; this seems like a bogus warning. In machine-generated
> code (such as the output from a macro), there can easily be expressions
> of the form "&x != NULL" which will always evaluate as true.
>
> > > #define ohci_dbg_sw(ohci, next, size, format, arg...) \
> > > do { \
> > > - if (next != NULL) { \
> > > unsigned s_len; \
> > > s_len = scnprintf (*next, *size, format, ## arg ); \
> > > *size -= s_len; *next += s_len; \
> > > - } else \
> > > - ohci_dbg(ohci,format, ## arg ); \
> > > } while (0);
>
> Nope. There can be situations where next _is_ NULL. For example, see
> ohci_dump() -> ohci_dump_roothub() later on.
>
> > I didn't look very deep, but another solution would be the one below:
> >
> > diff --git a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c
> > index d7d3449..5433010 100644
> > --- a/drivers/usb/host/ohci-dbg.c
> > +++ b/drivers/usb/host/ohci-dbg.c
> > @@ -639,7 +639,7 @@ static ssize_t fill_registers_buffer(struct debug_buffer *buf)
> >
> > /* dump driver info, then registers in spec order */
> >
> > - ohci_dbg_sw (ohci, &next, &size,
> > + ohci_dbg_sw (ohci, next, &size,
>
> That's definitely wrong. The type of the second argument must be
> (char **).
>
> The proper fix is probably to make ohci_dbg_sw an inline routine
> instead of a macro.
hmmm, makes sense. Something like below ? I'm not sure about the usage
of size though...
From dec2df46281ba1154fd7e5a05c9b31197c77de9d Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Thu, 10 Nov 2011 09:23:13 +0200
Subject: [PATCH] usb: host: ohci: fix compile warning
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Texas Instruments\n
Fix the following compile warning:
| In file included from drivers/usb/host/ohci-hcd.c:101:0:
| drivers/usb/host/ohci-dbg.c: In function ‘fill_registers_buffer’:
| drivers/usb/host/ohci-dbg.c:642:2: warning: the comparison will \
| always evaluate as ‘true’ for the address of ‘next’ will \
| never be NULL [-Waddress]
| drivers/usb/host/ohci-dbg.c:661:3: warning: the comparison will \
| always evaluate as ‘true’ for the address of ‘next’ will \
| never be NULL [-Waddress]
Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
drivers/usb/host/ohci-dbg.c | 30 ++++++++++++++++++++----------
1 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c
index d7d3449..ba7e29b 100644
--- a/drivers/usb/host/ohci-dbg.c
+++ b/drivers/usb/host/ohci-dbg.c
@@ -72,16 +72,26 @@ urb_print(struct urb * urb, char * str, int small, int status)
#endif
}
-#define ohci_dbg_sw(ohci, next, size, format, arg...) \
- do { \
- if (next != NULL) { \
- unsigned s_len; \
- s_len = scnprintf (*next, *size, format, ## arg ); \
- *size -= s_len; *next += s_len; \
- } else \
- ohci_dbg(ohci,format, ## arg ); \
- } while (0);
-
+static void ohci_dbg_sw(struct ohci_hcd *ohci, char **next,
+ unsigned *size, const char *fmt, ...)
+{
+ va_list args;
+
+ va_start(args, fmt);
+ do {
+ if (next != NULL) {
+ unsigned s_len;
+
+ s_len = scnprintf(*next, *size, fmt, args);
+ *size -= s_len;
+ *next += s_len;
+ } else {
+ ohci_dbg(ohci, fmt, args);
+ break;
+ }
+ } while(*size);
+ va_end(args);
+}
static void ohci_dump_intr_mask (
struct ohci_hcd *ohci,
--
1.7.8.rc0
--
balbi
Attachment:
signature.asc
Description: Digital signature
