On Jan 27, 2014, at 12:18 PM, Igor Galić <i.ga...@brainsware.org> wrote:
> > > ----- Original Message ----- >> Updated Branches: >> refs/heads/master 6215bf9e9 -> 77e2776ba >> >> >> TS-2519: make using RECP_NULL a compilation error >> >> RECP_NULL can still be useful to represent the case when a record >> does not have a persistence type. It might be a configuration record, >> for example. However, stats records shold never be registered as >> RECP_NULL, because that's ambiguous as to whether it should be >> persisted This change add some template helpers to ensure that any >> attempt to register a RECP_NULL stat dies in a horrible shower of >> compiler errors. > > Shouldn't this go on 5.0.x? There's no compatibility change here. >> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo >> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7eeb5c78 >> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7eeb5c78 >> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7eeb5c78 >> >> Branch: refs/heads/master >> Commit: 7eeb5c782b2f49a99d5e0ec62599f8c171fa7ea1 >> Parents: 7352493 >> Author: James Peach <jpe...@apache.org> >> Authored: Wed Jan 22 21:47:07 2014 -0800 >> Committer: James Peach <jpe...@apache.org> >> Committed: Mon Jan 27 09:30:14 2014 -0800 >> >> ---------------------------------------------------------------------- >> lib/records/I_RecCore.h | 14 ++++++++++---- >> lib/records/I_RecDefs.h | 25 +++++++++++++++++++++++++ >> lib/records/I_RecProcess.h | 4 +++- >> lib/records/P_RecCore.cc | 8 ++++---- >> lib/records/RecProcess.cc | 2 +- >> 5 files changed, 43 insertions(+), 10 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7eeb5c78/lib/records/I_RecCore.h >> ---------------------------------------------------------------------- >> diff --git a/lib/records/I_RecCore.h b/lib/records/I_RecCore.h >> index 424b1c2..e9b1a4f 100644 >> --- a/lib/records/I_RecCore.h >> +++ b/lib/records/I_RecCore.h >> @@ -73,11 +73,17 @@ const char * RecConfigOverrideFromEnvironment(const char >> * name, const char * va >> //------------------------------------------------------------------------- >> // Stat Registration >> //------------------------------------------------------------------------- >> -int RecRegisterStatInt(RecT rec_type, const char *name, RecInt data_default, >> RecPersistT persist_type); >> -int RecRegisterStatFloat(RecT rec_type, const char *name, RecFloat >> data_default, RecPersistT persist_type); >> -int RecRegisterStatString(RecT rec_type, const char *name, RecString >> data_default, RecPersistT persist_type); >> -int RecRegisterStatCounter(RecT rec_type, const char *name, RecCounter >> data_default, RecPersistT persist_type); >> +int _RecRegisterStatInt(RecT rec_type, const char *name, RecInt >> data_default, RecPersistT persist_type); >> +#define RecRegisterStatInt(rec_type, name, data_default, persist_type) >> _RecRegisterStatInt((rec_type), (name), (data_default), >> REC_PERSISTENCE_TYPE(persist_type)) >> >> +int _RecRegisterStatFloat(RecT rec_type, const char *name, RecFloat >> data_default, RecPersistT persist_type); >> +#define RecRegisterStatFloat(rec_type, name, data_default, persist_type) >> _RecRegisterStatFloat((rec_type), (name), (data_default), >> REC_PERSISTENCE_TYPE(persist_type)) >> + >> +int _RecRegisterStatString(RecT rec_type, const char *name, RecString >> data_default, RecPersistT persist_type); >> +#define RecRegisterStatString(rec_type, name, data_default, persist_type) >> _RecRegisterStatString((rec_type), (name), (data_default), >> REC_PERSISTENCE_TYPE(persist_type)) >> + >> +int _RecRegisterStatCounter(RecT rec_type, const char *name, RecCounter >> data_default, RecPersistT persist_type); >> +#define RecRegisterStatCounter(rec_type, name, data_default, persist_type) >> _RecRegisterStatCounter((rec_type), (name), (data_default), >> REC_PERSISTENCE_TYPE(persist_type)) >> >> //------------------------------------------------------------------------- >> // Config Registration >> >> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7eeb5c78/lib/records/I_RecDefs.h >> ---------------------------------------------------------------------- >> diff --git a/lib/records/I_RecDefs.h b/lib/records/I_RecDefs.h >> index 5a9c121..ec3afbc 100644 >> --- a/lib/records/I_RecDefs.h >> +++ b/lib/records/I_RecDefs.h >> @@ -88,6 +88,31 @@ enum RecPersistT >> RECP_NON_PERSISTENT >> }; >> >> +// RECP_NULL should never be used by callers of RecRegisterStat*(). You have >> to decide >> +// whether to persist stats or not. The template goop below make sure that >> passing RECP_NULL >> +// is a very ugle compile-time error. > > s/ugle/ugly/ > > Wouldn't a #pragma error have done it too? No, I don't think so. That's really only useful for #ifdeffery and that can't work for not allowing particular enum values in a specific context. J