[BHND] Add logging macroses
ClosedPublic
Actions

Authored by mizhka on May 6 2016, 5:22 PM.

Details

Reviewers

landon_landonf.org
adrian

Commits

rS299990: [bhnd] Add logging macros to BHND.

Summary

There are 5 logging levels:

ERROR
WARN
INFO
DEBUG
TRACE

There are 2 logging context:

with
without device

DEBUG and TRACE records are printed only if bootverbose
Logging records are printed with source code line information if acceptable logging level is DEBUG or TRACE

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

mizhka updated this revision to Diff 15983.May 6 2016, 5:22 PM

mizhka retitled this revision from to [BHND] Add logging macroses.

mizhka updated this object.

mizhka edited the test plan for this revision. (Show Details)

mizhka added reviewers: landon_landonf.org, adrian.

mizhka set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptMay 6 2016, 5:22 PM

mizhka updated this object.May 6 2016, 5:24 PM

Can you do a style(9) rework of this one?

Relevant style(9) guidelines:

Put a single tab character between the #define and the macro name.
The comment for #endif should match the expression used in the corresponding #if or #ifdef (e.g. #endif /* BHND_LOGGING >= BHND_WARN_LEVEL */)
Try to wrap these longer defines to keep lines <= 80 chars
Multi-statement macros should be encapsulated in a do { } while(0) loop

Lastly, a non-style(9) request:

Can you rework these to use C string literal concatenation and varargs to allow format strings? e.g., something like:

#define	BHND_LOG_PRINT(level, fmt...)	\
    printf("[BHND " level "] => %s:%d: " fmt,, __func__, __LINE__, ## __VA_ARGS__)

This revision now requires changes to proceed.May 6 2016, 9:10 PM

landon_landonf.org added inline comments.May 6 2016, 9:14 PM

sys/dev/bhnd/bhnd_debug.h
31	I'm not sure style(9) actually defines this, but #if-style header guards should be used here (e.g. `#ifndef _BHND_DEBUG_H_`)

imp added inline comments.May 6 2016, 9:31 PM

sys/dev/bhnd/bhnd_debug.h
31	#pragma once isn't standardized. And the tree is split between _FILE_ and _DIR_FILE_ for the guard name.

landon_landonf.org mentioned this in D6250: [BHND] ChipCommon: Resource managers, Serial & Parallel Flash support, .May 6 2016, 10:43 PM

mizhka updated this revision to Diff 16041.May 9 2016, 10:12 AM

mizhka edited edge metadata.

mizhka marked 2 inline comments as done.

removed #pragma once
added VA_ARGS usage
level is C concatened instead printf formatting

landon_landonf.org requested changes to this revision.May 10 2016, 12:22 AM

landon_landonf.org edited edge metadata.

landon_landonf.org added inline comments.

sys/dev/bhnd/bhnd_debug.h
48	These still need do {} while(0) loops around multiple statements, and line-breaks at 80 chars. You can avoid the trailing `printf("\n")` (and the need for do {} while(0) on some of the defines) if you go with something like: printf("bhnd: " fmt "\n", ## __VA_ARGS__);