[prev in list] [next in list] [prev in thread] [next in thread] 

List:       netbsd-tech-kern
Subject:    re: aprint_* used outside autoconfiguration step
From:       christos () zoulas ! com (Christos Zoulas)
Date:       2019-02-19 22:09:52
Message-ID: 20190219220952.62A5217FDA1 () rebar ! astron ! com
[Download RAW message or body]

On Feb 20,  8:44am, mrg@eterna.com.au (matthew green) wrote:
-- Subject: re: aprint_* used outside autoconfiguration step

| i don't like that patch for two reasons:
| 
| - if config_pending must be exposed, do it properly, not with a
|   another extern that isn't seen by the definition.  subr_prf.c
|   is already gross enough in that way.  however, i think it is
|   an abuse of it to use it this way, early autoconfig can happen
|   without ever increasing config_pending -- it all depends upon
|   what devices you have configured and what are present.

Yes, I don't like that either (using this variable). But introducing
another variable is more intrusive and at this point I would prefer
to re-evaluate how driver messaging is done.

| - no prefix at all seems worse.  at least i know it was an error
|   before, but no there is no context, just a message.
| 
| however, the biggest problem, IMO, is the presence of the API
| aprint_error() -- it takes no "device" parameter for a name, and
| thus is prefixless in messages making them confusing.
| 
| can we eliminate this one entirely while we are at it?  it's
| unfortunately used a *lot*.

There is aprint_debug() aprint_naive() and aprint_normal() too that
are prefix-less. Perhaps we should enforce a prefix name in case
where there is no ethernet interface or device involved, something
like the "driver name".

| > 	2. We don't have a non-autoconfig-related family of printf
| > 	   calls to handle errors outside autoconfiguration.
| 
| we have device_printf(9).  perhaps a device_printf_error()?

I would rename that to: printf_dev(), printf_error_dev() and
printf_error_ifnet() to be orthogonal; then again it is going to
be confusing with the semantics of LOG/CONS in the different aprint
variants (it is implicit with the variant where the data goes).
It also does not handle the debug/naive/normal cases. This is a mess.

christos
[prev in list] [next in list] [prev in thread] [next in thread]