On Mon, Apr 01, 2019 at 09:30:00AM -0400, Jeff King wrote:
> On Mon, Apr 01, 2019 at 01:52:16PM +0200, SZEDER Gábor wrote:
>
> > diff --git a/progress.c b/progress.c
> > index 842db14b72..3149ecd460 100644
> > --- a/progress.c
> > +++ b/progress.c
> > @@ -84,6 +84,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
> > const char *tp;
> > struct strbuf *counters_sb = &progress->counters_sb;
> > int show_update = 0;
> > + int last_count_len = counters_sb->len;
>
> I don't think it could matter here, as these are meant to be smallish
> strings, but I think we should get into the habit of using size_t
> consistently to hold string lengths.
>
> It makes auditing for integer overflow problems much simpler (this is on
> my mind as I happen to be tracing some bugs around this the past few
> days).
>
> (There are a few instances in the next patch, too. Other than this nit,
> though, your series looks good to me).
I started with using size_t, but then switched to int, because:
- After a bit of arithmetic I had to compare to term_columns()'s int
return value anyway (in the next patch).
- The second hunk in this patch adds these lines:
int clear_len = counters_sb->len < last_count_len ?
last_count_len - counters_sb->len : 0;
fprintf(stderr, "%s: %s%-*s", progress->title,
counters_sb->buf, clear_len, eol);
Here 'clear_len' has to be int, because the printf() format "%-*s"
expects an int, and otherwise -Werror=format= errors ensue. OK,
it could be size_t, but then it must be casted to an int upon
passing it to fprintf(), and after the next patch there will be
three such calls.
I could resend using size_t. Should I resend using size_t? :)
