Re: [PATCH v2 3/4] progress: clear previous progress update dynamically

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Apr 1, 2019, 14:15 UTC
Message-ID: <20190401141542.GJ32732@szeder.dev>
In-Reply-To: <20190401132959.GB11664@sigill.intra.peff.net>

On Mon, Apr 01, 2019 at 09:30:00AM -0400, Jeff King wrote:

Show 22 quoted lines

> 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? :)