Re: [bug-grep] length of dec. representation of a number

[Julian Foad]

Stepan Kasal wrote:
>         * src/grep.c (print_offset_sep, get_nondigit_option): Use a better
>           estimate for the length of the decimal representation of the
>           number.

> -  char buf[sizeof pos * CHAR_BIT];

Certainly "CHAR_BIT" was inappropriate and misleading - the number of bits in a 
char has nothing to do with the number of characters needed.  And it was 
wasting about 50 bytes of stack space, though that's not a problem as a one-off 
occurrence.

> +  char buf[(sizeof pos + 1) / 2 * 5 + 1];  /* log_10(2^16) < 5 */

Your expression is not _very_ complicated, but it took me at least 30 seconds 
to read the whole line and then work through it in my head, and that is really 
a needless distraction when reading the code.  It happens to give the exact 
length for the common sizes in use today (16, 32 and 64 bits) but a cruder 
estimate would be perfectly adequate and significantly easier to read:

  char buf[sizeof pos * 3 + 1];  /* 3 digits per byte is sufficient */

Additionally, as it appears twice already, it would be good to make it a macro:

  #define DIGITS_FOR(expr) (sizeof(expr) ...
  ...
  char buf[DIGITS_FOR(pos) + 1];
  ...

> @@ -1284,7 +1284,7 @@ static int
>  get_nondigit_option (int argc, char *const *argv, int *default_context)
>  {
>    int opt;
> -  char buf[sizeof (uintmax_t) * CHAR_BIT + 4];
> +  char buf[(sizeof(uintmax_t) + 1) / 2 * 5 + 4];  /* log_10(2^16) < 5 */

If you're optimising the buffer size, there's something else you should do. 
Here, the result is going to be stored in *default_context, an "int".  There's 
no point in allowing a number as big as "uintmax_t".  (The fact that 
context_length_arg() uses "uintmax_t" internally is irrelevant.)

  char buf[sizeof(*default_context) ...

- Julian