Date: Wed, 05 Nov 2003 17:17:17 -0500 From: "Brian F. Feldman" <green@freebsd.org> To: Sam Leffler <sam@errno.com> Cc: arch@freebsd.org Subject: Re: __VA_ARGS__izing IEEE80211_DPRINTF[2]() Message-ID: <200311052217.hA5MHHmx004511@green.bikeshed.org> In-Reply-To: Message from Sam Leffler <sam@errno.com> of "Wed, 05 Nov 2003 14:01:27 PST." <200311051401.27579.sam@errno.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Sam Leffler <sam@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@FreeBSD.org \ The Power to Serve! \
Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311052217.hA5MHHmx004511>