Hi Torsten,
On Mon, 3 Apr 2017, Torsten Bögershausen wrote:
> On 02/04/17 21:06, Johannes Schindelin wrote:
> > In its `atom_value` struct, the ref-filter source code wants to store
> > different values in a field called `ul` (for `unsigned long`), e.g.
> > timestamps.
> >
> > However, as we are about to switch the data type of timestamps away from
> > `unsigned long` (because it may be 32-bit even when `time_t` is 64-bit),
> > that data type is not large enough.
> >
> > Simply change that field to use `uintmax_t` instead.
> >
> > This patch is a bit larger than the mere change of the data type
> > because the field's name was tied to its data type, which has been fixed
> > at the same time.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > ref-filter.c | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/ref-filter.c b/ref-filter.c
> > index 9c82b5b9d63..8538328fc7f 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -351,7 +351,7 @@ struct ref_formatting_state {
> > struct atom_value {
> > const char *s;
> > void (*handler)(struct atom_value *atomv, struct ref_formatting_state
> > *state);
> > - unsigned long ul; /* used for sorting when not FIELD_STR */
> > + uintmax_t value; /* used for sorting when not FIELD_STR */
> > struct used_atom *atom;
> > };
> >
> > @@ -723,7 +723,7 @@ static void grab_common_values(struct atom_value *val,
> > int deref, struct object
> > if (!strcmp(name, "objecttype"))
> > v->s = typename(obj->type);
> > else if (!strcmp(name, "objectsize")) {
> > - v->ul = sz;
> > + v->value = sz;
> > v->s = xstrfmt("%lu", sz);
> > }
> > else if (deref)
> > @@ -770,8 +770,8 @@ static void grab_commit_values(struct atom_value *val,
> > int deref, struct object
> > v->s = xstrdup(oid_to_hex(&commit->tree->object.oid));
> > }
> > else if (!strcmp(name, "numparent")) {
> > - v->ul = commit_list_count(commit->parents);
> > - v->s = xstrfmt("%lu", v->ul);
> > + v->value = commit_list_count(commit->parents);
> > + v->s = xstrfmt("%lu", (unsigned long)v->value);
>
> If we want to get rid of "%lu" at some day, we can do like this:
> v->s = xstrfmt("%" PRIuMAX, v->value);
> Or, to make clear that under all circumstances an unsigned long is big enough
> to
> hold the counter, for readers in the future, use something like this:
> v->s = xstrfmt("%lu", (xulong_t)v->value);
We could do that, yes.
But part of my patch series is to clarify in a semantic way what the
purpose of the code is. Your solution would keep it syntactically correct,
of course, but it would also make it semantically unclear again.
By writing "%"PRIutime *instead of* "%"PRIuMAX, we are saying: look, we
are talking about a timestamp here. That would not at all be clear if we
wrote "%"PRIuMAX.
Ciao,
Johannes