__VA_ARGS__izing IEEE80211_DPRINTF[2]()

[Brian F. Feldman]

Sam Leffler <sam at errno.com> wrote:
> On Wednesday 05 November 2003 10:04 am, Brian Fundakowski Feldman wrote:
> > Would it be a problem to make the following change to src/sys/net80211 so
> > that the debug messages aren't totally useless for systems that have more
> > than one card (or confusing on systems that just have one)?  Obviously, it
> > would also involve removing the extra parentheses in each of the callers as
> > well.
> >
> > Old:
> > #define IEEE80211_DPRINTF(X)    if (ieee80211_debug) printf X
> > #define IEEE80211_DPRINTF2(X)   if (ieee80211_debug>1) printf X
> >
> > New:
> > #define	IEEE80211_DPRINTF(...)	do {						\
> > 	if (ieee80211_debug)							\
> > 		if_printf(&ic->ic_ifp, __VA_ARGS__);				\
> > while (0)
> >
> > The only place this wouldn't work is ieee80211_decap(), so I'd change it to
> > add a local "ic" variable when compiled for debugging.  There's an easy
> > fallback for non-C99 compilers, too; it just wouldn't print the interface:
> >
> > static __inline void
> > IEEE80211_DPRINTF(const char *fmt, ...)
> > {
> >
> > 	if (ieee80211_debug) {
> > 		va_list ap;
> >
> > 		va_start(ap, fmt);
> > 		(void)vprintf(fmt, ap);
> > 		va_end(ap);
> > 	}
> > }
> 
> I can't see what your intent is from the above.  If the point is to use 
> if_printf everywhere so all the printfs have a device prepended to the 
> message then I'm fine with that.  However I think it's a bad idea to depend 
> on local variables existing.  If you're going to do it, then add an explicit 
> argument to the macros.
> 
> If you're trying to deal with debugging systems w/ multiple 802.11 cards then 
> you probably want debugging enabled on a per-if basis which this doesn't 
> address.
> 
> Regardless, in all this remember that this code is shared with other systems 
> so changes like this shouldn't be done lightly. 

My intent is for the use of if_printf to prepend the interface name, yes.  
Per-interface debug flags will be trivial if we convert calls to say 
DPRINTF(ic, "fmt",...).  Not using the "magic" ic is fine with me -- just 
seems a shame because it's pervasive in just about every function you'd ever 
want to DPRINTF() from.

-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green at FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\