You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Igor Galić <i....@brainsware.org> on 2014/01/27 21:18:56 UTC
Re: [1/5] git commit: TS-2519: make using RECP_NULL a compilation
error
----- 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?
> 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 <jp...@apache.org>
> Authored: Wed Jan 22 21:47:07 2014 -0800
> Committer: James Peach <jp...@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?
> +
> +namespace rec {
> +namespace detail {
> +template <RecPersistT>
> +struct is_valid_persistence;
> +
> +template<>
> +struct is_valid_persistence<RECP_PERSISTENT>
> +{
> + static const RecPersistT value = RECP_PERSISTENT;
> +};
> +
> +template<>
> +struct is_valid_persistence<RECP_NON_PERSISTENT>
> +{
> + static const RecPersistT value = RECP_NON_PERSISTENT;
> +};
> +
> +}}
> +
> +#define REC_PERSISTENCE_TYPE(P) rec::detail::is_valid_persistence<P>::value
> +
> enum RecUpdateT
> {
> RECU_NULL, // default: don't know the behavior
>
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 8716 7A9F 989B ABD5 100F 4008 F266 55D6 2998 1641
Re: [1/5] git commit: TS-2519: make using RECP_NULL a compilation error
Posted by James Peach <jp...@apache.org>.
On Jan 27, 2014, at 12:18 PM, Igor Galić <i....@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 <jp...@apache.org>
>> Authored: Wed Jan 22 21:47:07 2014 -0800
>> Committer: James Peach <jp...@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