----- Original Message -----
> [ ... strange argument for not supporting high-res timers ... ]
> [...] but I'd rather not belabour the point.
Yes, please lets not belabour anything minor like this, it so obviously
needs to be implemented. I'll write this code so we can move on.
> > > > 2. Sentinel values
> > > >
> > > > I'm not so much against sentinels in the way Mark seemed to be, as
> > > > it looks kinda handy, but the choice of zero as a sentinel integer
> > > > value strikes me as questionable. [...prefer -1...]
> > >
> > > OK, will look into that. [...]
>
> A second thought appears here. We support unsigned integer types, for
> whom a -1 sentinel becomes an inconveniently large number. The
> sentinel-checking logic would have to be metric-type specific and IMHO
> unsightly, whereas zero is OK everywhere.
Except for double/float. And Unsightly is trumped by Misleading.
> Remember, the point of sentinel values is to be convenient for
> computation & output, should the application *not care* about the
> presence of errors. (If the application cared about errors, it would
> use individual status integers.) So what matters is not how easily
> the sentinel can be identified, but rather how smoothly an *unchecked*
> sentinel value would mesh in with real values, with minimal disruption
> of application data flow.
Firstly, reporting incorrect information is a big problem. And zero
as a numeric value is so often of meaning & importance that no matter
how uncaring a tool author is about ENODATA, we cannot encourage it.
Consider many values from the real world - swap.used, filesys.free,
mem.util.{free,dirty,...}, the many metrics that are indicate errors,
and on and on - zero really is the worst possible sentinel choice.
Secondly, above rationale is the inconsistent with the approach taken
for double/float which generates NaN instead of 0.0. We need to pick
one or the other, not half-and-half, and its important to not provide
potentially-very-misleading zero values as defaults.
It makes most sense to me to have the sentinel values as: empty string,
NaN (as you have), and all-bits-set for integers (ie -1). Then perhaps
provide a helper to aid with this programmer inconvenience angle:
int
pmTestFetchGroupSentinel_numeric(pmAtomValue *atom)
{
static struct pmAtomValue allset = { .ll = -1; }
return memcmp(atom, &allset, sizeof(*atom));
}
OTOH, is that overkill and catering too much for a perceived problem?
If the programmer is truly uncaring, they can just print out unsigned
values as signed (%d - they probably will be anyway, those lazy sods),
and the -1 will give a good, clear indicator that a sentinel is in use
(like "-nan" will for %f, which I think is a fine choice of sentinel).
>
> I think you're right on this one. (If removed, there will be
> qa/src/fetchgroup.c impact.)
>
OK, I'll see if I can fit that in for this release too.
cheers.
--
Nathan
|