On Feb 20, 8:44am, m...@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