--gVoQHMRaLt/xBMav
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 16, 2007 at 10:06:39AM +0000, Iain Hibbert wrote:
> Hi,
>    I thought a while ago that it would be nice to standardize DPRINTF and
> DPRINTFN, and now that we have a <sys/debug.h> I've been pondering how we
> could integrate them, and make it easier to generate debug output. These
> are my thoughts so far:
>=20
> 1. Add a keyword specification to config(5)
>=20
> 	debug <attribute> [<level>]
>=20
>    where <attribute> is the attribute that gets a file included, such as a
>    device name or system module such as "bluetooth", and <level> is the
>    default debug level for that attribute.

You don't mention where you intend to put such lines, in the
specification part (files.*) or in the user-edited selection part.

Guts says specification part (IMO, that's where it belongs anyway).

But...

> 2. config(1) will, when it sees this in a config file, do the following:
>=20
>   a. add the following to a generated debug.c:
>=20
> 	int debug_<attribute> =3D <level?=3D0>;
> 	sysctl_createv(clog, 0, NULL, NULL,
> 		CTLFLAG_PERMANENT | CTLFLAG_READWRITE,

[You want CTLFLAG_IMMEDIATE there, actually.]

> 		CTLTYPE_INT, "<attribute>",
> 		SYSCTL_DESCR("<attribute> debug level"),
> 		NULL, 0,
> 		&debug_<attribute>, sizeof(debug_<attribute>),
> 		CTL_DEBUG, CTL_CREATE, CTL_EOL);
>=20
>   b. add the following (or equivalent?) to the generated Makefile, for
>      files included by the attribute:
>=20
> 	COPTS.<file>+=3D"-DDEBUG=3Ddebug_<attribute>"

=2E.. that part is weird in that respect.  The way the define is made, and
peeking over next point, it seems your intent is to have the user put it
in the config file.

> 3. <sys/debug.h> contains something like:
>=20
> 	#ifdef DEBUG
> 	extern int DEBUG;
> 	#define DPRINTF(fmt, args...)	do {			\
> 		if ((DEBUG) > 0)				\
> 			printf("%s: "fmt, __func__ , ##args);	\
> 	} while (/* CONSTCOND */0)
>=20
> 	#define DPRINTFN(n, fmt, args...)	do {		\
> 		if ((DEBUG) > (n))				\
> 			printf("%s: "fmt, __func__ , ##args);	\
> 	} while (/* CONSTCOND */0)
> 	#else
> 	#define DPRINTF(...)
> 	#define DPRINTFN(...)
> 	#endif

I think a slightly different version would be better, and address most
ofthe issues you mention:

COPTS.<file>+=3D  "-DDPRINTATTR=3D<attribute>"

#ifdef DEBUG /* The real thing, i.e. 'options DEBUG' */

# ifdef DPRINTATTR

#  define DPRINTF(fmt, args...) do { \
    if ((__CONCAT(debug_,DPRINTATTR)) > 0) \
        printf(...); while (0)
#  define DPRINTFN(n, fmt, args...) [...]

# else /* DPRINTATTR */

#  define DPRINTF(fmt, args...) do { \
    if (debug_kernel > 0) [...]
#  define DPRINTFN [...]

# endif /* DPRINTATTR */

#else /* DEBUG */

# define DPRINTF(...)
# define DPRINTFN(...)

#endif /* DEBUG */

What this gives is:

1. subsystems define their debug stuff in files.whatever, and it gets
   brought in along with the sysctl hierarchy when user do 'options
   DEBUG'.  Note that the user still has fine-grained control over the
   messages because it does the selection through sysctl.  However,
   messages in all subsystems are potentially available.

2. files that don't depend on an attribute will depend on debug_kernel.
   It's up to init_sysctl.c to provide the node for that one.  That part
   is a bit of a problem, since setting debug_kernel potentially
   activates a lot of messages.  However, having "debug <file>" in
   config(5) is probably the best option in that case.

It remains the issue of files that depend on several attributes, each
with their own debug stuff.  Here I should say that I really dislike the
! operator in attribute logic for file selection, for a different reason.
Collecting attr1 | attr2 is very easy (create an attribute that both
attr1 and attr2 will depend on).  I'd have to think about attr1 & attr2,
but in the end, for the sake of proper modularisation, I really wish
files only depended on a single attribute.

[...]
> is this desireable?

It'd be nice to have, I think.

> any other comments before I look at implementation?

I can definitely help with the config(1) part.

--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"You could have made it, spitting out benchmarks
Owe it to yourself not to fail"
Amplifico, Spitting Out Benchmarks, Hometakes Vol. 2, 2005.

--gVoQHMRaLt/xBMav
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (NetBSD)

iQEVAwUBRfrIf9goQloHrPnoAQIvUAf/V8ile406ze9mQBWNvh0ejyetdhiYiRVT
534vyfM6QO9oVMMvA9aXCklMiuBei6RtkWHGhBok3q2RAqvFWmk8dBVpDuxDYUz8
NffC6HOr4wR90RA80m1biDB760apUyrd3zzeGezNIluAcJgpDL7EIgK8sM4VxjYs
YI/zLNgVienCth2B2fIqeeUXy+ad6uh6XCjM/az+ONPNpV5BEOK/5eevlW08xEFb
ZK5dyp+FImiZs3QRUJj4V+ia68/gzx0JA+s39/fpjH0a2vTw5VHyP2KOCR1osUpW
VJ/yj5Ckac+jvi5MH5XBtHmbBe6poMtrKSPgdSEJTmgUkSGLfHaePQ==
=rCei
-----END PGP SIGNATURE-----

--gVoQHMRaLt/xBMav--