Ander Juaristi <a@xxxxxxxxxxxx> wrote: > - Rename constants: TYPE_TIME_DATE, TYPE_TIME_HOUR, TYPE_TIME_DAY > - Use array_size() > - Rewrite __hour_type_print_r to get buffer size from a parameter Thanks! I think this can be squashed too. > --- a/src/meta.c > +++ b/src/meta.c > @@ -475,7 +475,7 @@ static void day_type_print(const struct expr *expr, struct output_ctx *octx) > "Saturday" > }; > uint8_t daynum = mpz_get_uint8(expr->value), > - numdays = sizeof(days) / (3 * 3); > + numdays = array_size(days) - 1; > > if (daynum >= 0 && daynum <= numdays) This '- 1' had me confused, but then you use <= numdays. Perhaps prefer array_size + "<" operator? > @@ -505,7 +505,7 @@ static struct error_record *day_type_parse(const struct expr *sym, > "Friday", > "Saturday" > }; > - int daynum = -1, numdays = (sizeof(days) / 7) - 1; > + int daynum = -1, numdays = array_size(days); ... because here the - 1 is missing. > if (cur_tm) > seconds = (seconds + cur_tm->tm_gmtoff) % 86400; > > - __hour_type_print_r(0, 0, seconds, out); > + __hour_type_print_r(0, 0, seconds, out, sizeof(out)); Thanks for doing this. I know its more code to read but it helps when someone has to review this later as this got rid of magic bufsize constants in the snprintf() calls. > .byteorder = BYTEORDER_HOST_ENDIAN, > - .size = 8 * BITS_PER_BYTE, > + .size = sizeof(uint64_t) * BITS_PER_BYTE, Thanks.