Hi,

For our use case, we end up performing a lot of string comparisons to
determine the state of the system via libchrony. Also, we must duplicate
string constants. This is due to message.c containing values that are
"hidden" from our application. Namely, the sources_state_enums:

https://gitlab.com/chrony/libchrony/-/blob/main/message.c#L67

Ideally, we'd be able to compare the integer values, rather than the
strings. This would enable us to drop the duplicated string constants and
no longer perform string comparisons.

In effect, what do you think about defining constants in a header file,
instead? Something along the lines of:

static const Constant sources_state_enums[] = { {
CHRONY_SOURCE_SELECTED, "Selected" },   { CHRONY_SOURCE_NONSELECTABLE,
"Nonselectable" },      { CHRONY_SOURCE_FALSE_TICKER, "Falseticker" },  {
CHRONY_SOURCE_JITTERY, "Jittery" },     { CHRONY_SOURCE_UNSELECTED,
"Unselected" }, { CHRONY_SOURCE_SELECTABLE, "Selectable" },     { 0 }};

Or perhaps even eliminating the dependency on strings altogether in the API
in favour of enumerated values? Offering a lookup for the values would
still be useful, and even internationalizable.

We do:

  get_field_uinteger( ... )
  get_constant_name( ... )

Then compare the constant name against our duplicated string values. We
could probably compare the value returned from get_field_uinteger, but we'd
still have to duplicate the constant values, which could lead to runtime
breakages rather than compile-time errors.

The following would be preferred:

  #include <message.h>
  chrony_state cs = get_field_state( ... )
  if( cs.value == CHRONY_SOURCE_SELECTED ) { ... }

Or possibly hide the query behind an API call, leading to a simpler check:

  // prototype for is_field_state and constant values
  #include <message.h>

  // query the state, provide the session, returns a Boolean
  if( is_field_state( ...,  CHRONY_SOURCE_SELECTED ) ) { ... }

Thoughts?

Reply via email to