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

Reply via email to