[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
2015-08-02 20:58, Wiles, Keith: > On 8/2/15, 3:44 PM, "Thomas Monjalon" wrote: > > >2015-08-02 19:10, Wiles, Keith: > >> On 8/2/15, 12:15 PM, "Thomas Monjalon" > >>wrote: > >> >2015-06-06 19:04, Keith Wiles: > >> >> +# Log level use: RTE_LOG_XXX > >> >> +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or > >>DEBUG > >> >> +# Look in rte_log.h for others if any. > >> >> +# > >> > > >> >I think this comment is useless. > >> > >> I do not think the comment is useless as some may not understand what > >> values the Log level can be set too in the future. Not commenting the > >> change would be a problem IMO. This is also why the line was moved. > > > >It is already documented in the API doc. > >I agree having some comments in the config files would be convenient but: > > - this one is 3 lines long > > - currently comments are only used to separate sections > >Maybe you can do a oneline: > > # Minimum log level compiled: DEBUG, INFO, NOTICE, WARNING or ERR > >I think it is important to tell it is a minimum log level, i.e. compiled > >logs. > >And probably it is not needed to suggest a minimum level higher than ERR. > > I will reduce the comment to one line, should I move the LOG_HISTORY down > to under LOG_LEVEL? Yes please. > >> >> + RTE_LOG_EMERG, /**< System is unusable. */ > >> >> + RTE_LOG_ALERT, /**< Action must be taken immediately. */ > >> >> + RTE_LOG_CRIT, /**< Critical conditions. */ > >> >> + RTE_LOG_ERR,/**< Error conditions. */ > >> >> + RTE_LOG_WARNING,/**< Warning conditions. */ > >> >> + RTE_LOG_NOTICE, /**< Normal but significant condition. */ > >> >> + RTE_LOG_INFO, /**< Informational.*/ > >> >> + RTE_LOG_DEBUG /**< Debug-level messages. */ > >> >> +}; > >> > > >> >What is the benefit of this change? > >> > >> The change is to use a enum in place of using magic numbers, plus you > >>get > >> the benefit of seeing the enum name in the debugger instead of a number. > >> It makes the code more readable IMHO. > > > >OK so a comment in the commit message could give the debugger > >justification. > > OK will add the debugger comment to the commit log. Thanks Please also explain that rte_logs was set after options parsing, defaulting to RTE_LOG_LEVEL, and it is now initialized at RTE_LOG_LEVEL without behavioral change.
[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
2015-08-02 19:10, Wiles, Keith: > On 8/2/15, 12:15 PM, "Thomas Monjalon" wrote: > >2015-06-06 19:04, Keith Wiles: > >> +# Log level use: RTE_LOG_XXX > >> +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or DEBUG > >> +# Look in rte_log.h for others if any. > >> +# > > > >I think this comment is useless. > > I do not think the comment is useless as some may not understand what > values the Log level can be set too in the future. Not commenting the > change would be a problem IMO. This is also why the line was moved. It is already documented in the API doc. I agree having some comments in the config files would be convenient but: - this one is 3 lines long - currently comments are only used to separate sections Maybe you can do a oneline: # Minimum log level compiled: DEBUG, INFO, NOTICE, WARNING or ERR I think it is important to tell it is a minimum log level, i.e. compiled logs. And probably it is not needed to suggest a minimum level higher than ERR. > >> +enum { > >> + RTE_LOG_NOOP = 0, /**< Noop not used (zero entry)*/ > > > >NOOP is useless: EMERG may be = 1 > > Does it really matter if I used RTE_LOG_NOOP, just to make sure someone > did not try to use zero here. Instead of setting the RTE_LOG_EMERG=1, I > can change it to be the way you suggest, but I think it does not hurt > anything does it? We avoid adding code without real use which could cause confusion. > >> + RTE_LOG_EMERG, /**< System is unusable. */ > >> + RTE_LOG_ALERT, /**< Action must be taken immediately. */ > >> + RTE_LOG_CRIT, /**< Critical conditions. */ > >> + RTE_LOG_ERR,/**< Error conditions. */ > >> + RTE_LOG_WARNING,/**< Warning conditions. */ > >> + RTE_LOG_NOTICE, /**< Normal but significant condition. */ > >> + RTE_LOG_INFO, /**< Informational.*/ > >> + RTE_LOG_DEBUG /**< Debug-level messages. */ > >> +}; > > > >What is the benefit of this change? > > The change is to use a enum in place of using magic numbers, plus you get > the benefit of seeing the enum name in the debugger instead of a number. > It makes the code more readable IMHO. OK so a comment in the commit message could give the debugger justification.
[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
On 8/2/15, 3:44 PM, "Thomas Monjalon" wrote: >2015-08-02 19:10, Wiles, Keith: >> On 8/2/15, 12:15 PM, "Thomas Monjalon" >>wrote: >> >2015-06-06 19:04, Keith Wiles: >> >> +# Log level use: RTE_LOG_XXX >> >> +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or >>DEBUG >> >> +# Look in rte_log.h for others if any. >> >> +# >> > >> >I think this comment is useless. >> >> I do not think the comment is useless as some may not understand what >> values the Log level can be set too in the future. Not commenting the >> change would be a problem IMO. This is also why the line was moved. > >It is already documented in the API doc. >I agree having some comments in the config files would be convenient but: > - this one is 3 lines long > - currently comments are only used to separate sections >Maybe you can do a oneline: > # Minimum log level compiled: DEBUG, INFO, NOTICE, WARNING or ERR >I think it is important to tell it is a minimum log level, i.e. compiled >logs. >And probably it is not needed to suggest a minimum level higher than ERR. I will reduce the comment to one line, should I move the LOG_HISTORY down to under LOG_LEVEL? > >> >> +enum { >> >> + RTE_LOG_NOOP = 0, /**< Noop not used (zero entry)*/ >> > >> >NOOP is useless: EMERG may be = 1 >> >> Does it really matter if I used RTE_LOG_NOOP, just to make sure someone >> did not try to use zero here. Instead of setting the RTE_LOG_EMERG=1, I >> can change it to be the way you suggest, but I think it does not hurt >> anything does it? > >We avoid adding code without real use which could cause confusion. I will remove NOOP and set EMERG=1. > >> >> + RTE_LOG_EMERG, /**< System is unusable. */ >> >> + RTE_LOG_ALERT, /**< Action must be taken immediately. */ >> >> + RTE_LOG_CRIT, /**< Critical conditions. */ >> >> + RTE_LOG_ERR,/**< Error conditions. */ >> >> + RTE_LOG_WARNING,/**< Warning conditions. */ >> >> + RTE_LOG_NOTICE, /**< Normal but significant condition. */ >> >> + RTE_LOG_INFO, /**< Informational.*/ >> >> + RTE_LOG_DEBUG /**< Debug-level messages. */ >> >> +}; >> > >> >What is the benefit of this change? >> >> The change is to use a enum in place of using magic numbers, plus you >>get >> the benefit of seeing the enum name in the debugger instead of a number. >> It makes the code more readable IMHO. > >OK so a comment in the commit message could give the debugger >justification. OK will add the debugger comment to the commit log. > > ? Regards, ++Keith Intel Corporation
[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
2015-06-06 19:04, Keith Wiles: > --- a/config/common_bsdapp > +++ b/config/common_bsdapp > @@ -93,12 +93,18 @@ CONFIG_RTE_MAX_NUMA_NODES=8 > CONFIG_RTE_MAX_MEMSEG=256 > CONFIG_RTE_MAX_MEMZONE=2560 > CONFIG_RTE_MAX_TAILQ=32 > -CONFIG_RTE_LOG_LEVEL=8 > CONFIG_RTE_LOG_HISTORY=256 > CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n > CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n > > # > +# Log level use: RTE_LOG_XXX > +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or DEBUG > +# Look in rte_log.h for others if any. > +# I think this comment is useless. > +CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG Yes, easier to read. Please do not move line without good reason. It was more logic to see it along with LOG_HISTORY. > --- a/lib/librte_eal/common/eal_common_log.c > +++ b/lib/librte_eal/common/eal_common_log.c > @@ -82,7 +82,7 @@ static struct log_history_list log_history; > /* global log structure */ > struct rte_logs rte_logs = { > .type = ~0, > - .level = RTE_LOG_DEBUG, > + .level = RTE_LOG_LEVEL, > .file = NULL, > }; OK, more consistent. It was set to RTE_LOG_LEVEL later anyway. (this comment would be useful in the commit message) > /* Can't use 0, as it gives compiler warnings */ > -#define RTE_LOG_EMERG1U /**< System is unusable. */ > -#define RTE_LOG_ALERT2U /**< Action must be taken immediately. */ > -#define RTE_LOG_CRIT 3U /**< Critical conditions. */ > -#define RTE_LOG_ERR 4U /**< Error conditions. */ > -#define RTE_LOG_WARNING 5U /**< Warning conditions. */ > -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */ > -#define RTE_LOG_INFO 7U /**< Informational.*/ > -#define RTE_LOG_DEBUG8U /**< Debug-level messages. */ > +enum { > + RTE_LOG_NOOP = 0, /**< Noop not used (zero entry)*/ NOOP is useless: EMERG may be = 1 > + RTE_LOG_EMERG, /**< System is unusable. */ > + RTE_LOG_ALERT, /**< Action must be taken immediately. */ > + RTE_LOG_CRIT, /**< Critical conditions. */ > + RTE_LOG_ERR,/**< Error conditions. */ > + RTE_LOG_WARNING,/**< Warning conditions. */ > + RTE_LOG_NOTICE, /**< Normal but significant condition. */ > + RTE_LOG_INFO, /**< Informational.*/ > + RTE_LOG_DEBUG /**< Debug-level messages. */ > +}; What is the benefit of this change?
[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
On 8/2/15, 2:10 PM, "dev on behalf of Wiles, Keith" wrote: >On 8/2/15, 12:15 PM, "Thomas Monjalon" wrote: > >>2015-06-06 19:04, Keith Wiles: >>> --- a/config/common_bsdapp >>> +++ b/config/common_bsdapp >>> @@ -93,12 +93,18 @@ CONFIG_RTE_MAX_NUMA_NODES=8 >>> CONFIG_RTE_MAX_MEMSEG=256 >>> CONFIG_RTE_MAX_MEMZONE=2560 >>> CONFIG_RTE_MAX_TAILQ=32 >>> -CONFIG_RTE_LOG_LEVEL=8 >>> CONFIG_RTE_LOG_HISTORY=256 >>> CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n >>> CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n >>> >>> # >>> +# Log level use: RTE_LOG_XXX >>> +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or >>>DEBUG >>> +# Look in rte_log.h for others if any. >>> +# >> >>I think this comment is useless. > >I do not think the comment is useless as some may not understand what >values the Log level can be set too in the future. Not commenting the >change would be a problem IMO. This is also why the line was moved. >> >>> +CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG >> >>Yes, easier to read. >>Please do not move line without good reason. It was more logic to see it >>along >>with LOG_HISTORY. Maybe moving LOG_HISTORY with LOG_LEVEL would have been a better option. > >Moving the line was for the comment and now it is a enum value instead of >a magic number. Magic numbers are bad right? Adding a comment to help the >user set this value is always reasonable IMO unless the comment is not >correct, is this the case? >> >>> --- a/lib/librte_eal/common/eal_common_log.c >>> +++ b/lib/librte_eal/common/eal_common_log.c >>> @@ -82,7 +82,7 @@ static struct log_history_list log_history; >>> /* global log structure */ >>> struct rte_logs rte_logs = { >>> .type = ~0, >>> - .level = RTE_LOG_DEBUG, >>> + .level = RTE_LOG_LEVEL, >>> .file = NULL, >>> }; >> >>OK, more consistent. >>It was set to RTE_LOG_LEVEL later anyway. >>(this comment would be useful in the commit message) >> >>> /* Can't use 0, as it gives compiler warnings */ >>> -#define RTE_LOG_EMERG1U /**< System is unusable. */ >>> -#define RTE_LOG_ALERT2U /**< Action must be taken immediately. */ >>> -#define RTE_LOG_CRIT 3U /**< Critical conditions. */ >>> -#define RTE_LOG_ERR 4U /**< Error conditions. */ >>> -#define RTE_LOG_WARNING 5U /**< Warning conditions. */ >>> -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */ >>> -#define RTE_LOG_INFO 7U /**< Informational.*/ >>> -#define RTE_LOG_DEBUG8U /**< Debug-level messages. */ >>> +enum { >>> + RTE_LOG_NOOP = 0, /**< Noop not used (zero entry)*/ >> >>NOOP is useless: EMERG may be = 1 > >Does it really matter if I used RTE_LOG_NOOP, just to make sure someone >did not try to use zero here. Instead of setting the RTE_LOG_EMERG=1, I >can change it to be the way you suggest, but I think it does not hurt >anything does it? >> >>> + RTE_LOG_EMERG, /**< System is unusable. */ >>> + RTE_LOG_ALERT, /**< Action must be taken immediately. */ >>> + RTE_LOG_CRIT, /**< Critical conditions. */ >>> + RTE_LOG_ERR,/**< Error conditions. */ >>> + RTE_LOG_WARNING,/**< Warning conditions. */ >>> + RTE_LOG_NOTICE, /**< Normal but significant condition. */ >>> + RTE_LOG_INFO, /**< Informational.*/ >>> + RTE_LOG_DEBUG /**< Debug-level messages. */ >>> +}; >> >>What is the benefit of this change? > >The change is to use a enum in place of using magic numbers, plus you get >the benefit of seeing the enum name in the debugger instead of a number. >It makes the code more readable IMHO. >> >> > >To me the code is fine and the only change would be the RTE_LOG_NOOP being >remove and RTE_LOG_EMERG=1. > >? >Regards, >++Keith >Intel Corporation > > > > ? Regards, ++Keith Intel Corporation
[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
On 8/2/15, 12:15 PM, "Thomas Monjalon" wrote: >2015-06-06 19:04, Keith Wiles: >> --- a/config/common_bsdapp >> +++ b/config/common_bsdapp >> @@ -93,12 +93,18 @@ CONFIG_RTE_MAX_NUMA_NODES=8 >> CONFIG_RTE_MAX_MEMSEG=256 >> CONFIG_RTE_MAX_MEMZONE=2560 >> CONFIG_RTE_MAX_TAILQ=32 >> -CONFIG_RTE_LOG_LEVEL=8 >> CONFIG_RTE_LOG_HISTORY=256 >> CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n >> CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n >> >> # >> +# Log level use: RTE_LOG_XXX >> +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or DEBUG >> +# Look in rte_log.h for others if any. >> +# > >I think this comment is useless. I do not think the comment is useless as some may not understand what values the Log level can be set too in the future. Not commenting the change would be a problem IMO. This is also why the line was moved. > >> +CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG > >Yes, easier to read. >Please do not move line without good reason. It was more logic to see it >along >with LOG_HISTORY. Moving the line was for the comment and now it is a enum value instead of a magic number. Magic numbers are bad right? Adding a comment to help the user set this value is always reasonable IMO unless the comment is not correct, is this the case? > >> --- a/lib/librte_eal/common/eal_common_log.c >> +++ b/lib/librte_eal/common/eal_common_log.c >> @@ -82,7 +82,7 @@ static struct log_history_list log_history; >> /* global log structure */ >> struct rte_logs rte_logs = { >> .type = ~0, >> -.level = RTE_LOG_DEBUG, >> +.level = RTE_LOG_LEVEL, >> .file = NULL, >> }; > >OK, more consistent. >It was set to RTE_LOG_LEVEL later anyway. >(this comment would be useful in the commit message) > >> /* Can't use 0, as it gives compiler warnings */ >> -#define RTE_LOG_EMERG1U /**< System is unusable. */ >> -#define RTE_LOG_ALERT2U /**< Action must be taken immediately. */ >> -#define RTE_LOG_CRIT 3U /**< Critical conditions. */ >> -#define RTE_LOG_ERR 4U /**< Error conditions. */ >> -#define RTE_LOG_WARNING 5U /**< Warning conditions. */ >> -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */ >> -#define RTE_LOG_INFO 7U /**< Informational.*/ >> -#define RTE_LOG_DEBUG8U /**< Debug-level messages. */ >> +enum { >> +RTE_LOG_NOOP = 0, /**< Noop not used (zero entry)*/ > >NOOP is useless: EMERG may be = 1 Does it really matter if I used RTE_LOG_NOOP, just to make sure someone did not try to use zero here. Instead of setting the RTE_LOG_EMERG=1, I can change it to be the way you suggest, but I think it does not hurt anything does it? > >> +RTE_LOG_EMERG, /**< System is unusable. */ >> +RTE_LOG_ALERT, /**< Action must be taken immediately. */ >> +RTE_LOG_CRIT, /**< Critical conditions. */ >> +RTE_LOG_ERR,/**< Error conditions. */ >> +RTE_LOG_WARNING,/**< Warning conditions. */ >> +RTE_LOG_NOTICE, /**< Normal but significant condition. */ >> +RTE_LOG_INFO, /**< Informational.*/ >> +RTE_LOG_DEBUG /**< Debug-level messages. */ >> +}; > >What is the benefit of this change? The change is to use a enum in place of using magic numbers, plus you get the benefit of seeing the enum name in the debugger instead of a number. It makes the code more readable IMHO. > > To me the code is fine and the only change would be the RTE_LOG_NOOP being remove and RTE_LOG_EMERG=1. ? Regards, ++Keith Intel Corporation
[dpdk-dev] [PATCH] log:Change magic number on RTE_LOG_LEVEL to a define
Config files used RTE_LOG_LEVEL=8 to set log level to DEBUG. Using a the RTE_LOG_ is easier to maintain. Converted the RTE_LOG_ defines into a enum of values with the same names for to reduct maintaining the values and allow debuggers to print the name of the value. Signed-off-by: Keith Wiles --- config/common_bsdapp| 8 +++- config/common_linuxapp | 8 +++- lib/librte_eal/common/eal_common_log.c | 4 ++-- lib/librte_eal/common/include/rte_log.h | 19 +++ 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/config/common_bsdapp b/config/common_bsdapp index 0b169c8..97bbcbd 100644 --- a/config/common_bsdapp +++ b/config/common_bsdapp @@ -93,12 +93,18 @@ CONFIG_RTE_MAX_NUMA_NODES=8 CONFIG_RTE_MAX_MEMSEG=256 CONFIG_RTE_MAX_MEMZONE=2560 CONFIG_RTE_MAX_TAILQ=32 -CONFIG_RTE_LOG_LEVEL=8 CONFIG_RTE_LOG_HISTORY=256 CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n CONFIG_RTE_EAL_ALWAYS_PANIC_ON_ERROR=n # +# Log level use: RTE_LOG_XXX +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or DEBUG +# Look in rte_log.h for others if any. +# +CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG + +# # FreeBSD contiguous memory driver settings # CONFIG_RTE_CONTIGMEM_MAX_NUM_BUFS=64 diff --git a/config/common_linuxapp b/config/common_linuxapp index 5deb55a..886fc66 100644 --- a/config/common_linuxapp +++ b/config/common_linuxapp @@ -93,7 +93,6 @@ CONFIG_RTE_MAX_NUMA_NODES=8 CONFIG_RTE_MAX_MEMSEG=256 CONFIG_RTE_MAX_MEMZONE=2560 CONFIG_RTE_MAX_TAILQ=32 -CONFIG_RTE_LOG_LEVEL=8 CONFIG_RTE_LOG_HISTORY=256 CONFIG_RTE_LIBEAL_USE_HPET=n CONFIG_RTE_EAL_ALLOW_INV_SOCKET_ID=n @@ -102,6 +101,13 @@ CONFIG_RTE_EAL_IGB_UIO=y CONFIG_RTE_EAL_VFIO=y # +# Log level use: RTE_LOG_XXX +# XXX = NOOP, EMERG, ALERT, CRIT, ERR, WARNING, NOTICE, INFO or DEBUG +# Look in rte_log.h for others if any. +# +CONFIG_RTE_LOG_LEVEL=RTE_LOG_DEBUG + +# # Special configurations in PCI Config Space for high performance # CONFIG_RTE_PCI_CONFIG=n diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c index fe3d7d5..3dcceab 100644 --- a/lib/librte_eal/common/eal_common_log.c +++ b/lib/librte_eal/common/eal_common_log.c @@ -82,7 +82,7 @@ static struct log_history_list log_history; /* global log structure */ struct rte_logs rte_logs = { .type = ~0, - .level = RTE_LOG_DEBUG, + .level = RTE_LOG_LEVEL, .file = NULL, }; @@ -93,7 +93,7 @@ static int history_enabled = 1; /** * This global structure stores some informations about the message - * that is currently beeing processed by one lcore + * that is currently being processed by one lcore */ struct log_cur_msg { uint32_t loglevel; /**< log level - see rte_log.h */ diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h index 3b467c1..e7e893e 100644 --- a/lib/librte_eal/common/include/rte_log.h +++ b/lib/librte_eal/common/include/rte_log.h @@ -89,14 +89,17 @@ extern struct rte_logs rte_logs; #define RTE_LOGTYPE_USER8 0x8000 /**< User-defined log type 8. */ /* Can't use 0, as it gives compiler warnings */ -#define RTE_LOG_EMERG1U /**< System is unusable. */ -#define RTE_LOG_ALERT2U /**< Action must be taken immediately. */ -#define RTE_LOG_CRIT 3U /**< Critical conditions. */ -#define RTE_LOG_ERR 4U /**< Error conditions. */ -#define RTE_LOG_WARNING 5U /**< Warning conditions. */ -#define RTE_LOG_NOTICE 6U /**< Normal but significant condition. */ -#define RTE_LOG_INFO 7U /**< Informational.*/ -#define RTE_LOG_DEBUG8U /**< Debug-level messages. */ +enum { + RTE_LOG_NOOP = 0, /**< Noop not used (zero entry)*/ + RTE_LOG_EMERG, /**< System is unusable. */ + RTE_LOG_ALERT, /**< Action must be taken immediately. */ + RTE_LOG_CRIT, /**< Critical conditions. */ + RTE_LOG_ERR,/**< Error conditions. */ + RTE_LOG_WARNING,/**< Warning conditions. */ + RTE_LOG_NOTICE, /**< Normal but significant condition. */ + RTE_LOG_INFO, /**< Informational.*/ + RTE_LOG_DEBUG /**< Debug-level messages. */ +}; /** The default log stream. */ extern FILE *eal_default_log_stream; -- 2.3.0