From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jeff King <peff@peff.net>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Duy Nguyen" <pclouds@gmail.com>,
"Luke Mewburn" <luke@mewburn.net>,
git@vger.kernel.org
Subject: Re: [PATCH v2 3/4] progress: clear previous progress update dynamically
Date: Mon, 1 Apr 2019 16:15:42 +0200 [thread overview]
Message-ID: <20190401141542.GJ32732@szeder.dev> (raw)
In-Reply-To: <20190401132959.GB11664@sigill.intra.peff.net>
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? :)
next prev parent reply other threads:[~2019-04-01 14:15 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-25 10:38 [PATCH 0/5] Progress display fixes SZEDER Gábor
2019-03-25 10:38 ` [PATCH 1/5] progress: make display_progress() return void SZEDER Gábor
2019-03-25 10:38 ` [PATCH 2/5] progress: return early when in the background SZEDER Gábor
2019-03-25 11:08 ` Ævar Arnfjörð Bjarmason
2019-03-25 11:39 ` SZEDER Gábor
2019-03-26 6:28 ` Luke Mewburn
2019-03-26 5:38 ` Jeff King
2019-03-25 10:38 ` [PATCH 3/5] progress: assemble percentage and counters in a strbuf before printing SZEDER Gábor
2019-03-26 5:45 ` Jeff King
2019-03-27 10:24 ` SZEDER Gábor
2019-03-28 2:12 ` Jeff King
2019-03-25 10:38 ` [PATCH 4/5] progress: clear previous progress update dynamically SZEDER Gábor
2019-03-25 10:38 ` [PATCH 5/5] progress: break too long progress bar lines SZEDER Gábor
2019-03-25 11:02 ` Duy Nguyen
2019-03-25 11:12 ` SZEDER Gábor
2019-03-26 5:52 ` [PATCH 0/5] Progress display fixes Jeff King
2019-04-01 11:52 ` [PATCH v2 0/4] " SZEDER Gábor
2019-04-01 11:52 ` [PATCH v2 1/4] progress: make display_progress() return void SZEDER Gábor
2019-04-02 5:42 ` Eric Sunshine
2019-04-01 11:52 ` [PATCH v2 2/4] progress: assemble percentage and counters in a strbuf before printing SZEDER Gábor
2019-04-02 5:45 ` Eric Sunshine
2019-04-02 5:50 ` Eric Sunshine
2019-04-01 11:52 ` [PATCH v2 3/4] progress: clear previous progress update dynamically SZEDER Gábor
2019-04-01 13:30 ` Jeff King
2019-04-01 14:15 ` SZEDER Gábor [this message]
2019-04-02 14:27 ` Jeff King
2019-04-01 11:52 ` [PATCH v2 4/4] progress: break too long progress bar lines SZEDER Gábor
2019-04-05 0:45 ` [PATCH v3 0/4] Progress display fixes SZEDER Gábor
2019-04-05 0:45 ` [PATCH v3 1/4] progress: make display_progress() return void SZEDER Gábor
2019-04-05 0:45 ` [PATCH v3 2/4] progress: assemble percentage and counters in a strbuf before printing SZEDER Gábor
2019-04-05 0:45 ` [PATCH v3 3/4] progress: clear previous progress update dynamically SZEDER Gábor
2019-04-05 0:45 ` [PATCH v3 4/4] progress: break too long progress bar lines SZEDER Gábor
2019-04-05 22:21 ` [PATCH v3 0/4] Progress display fixes Jeff King
2019-04-12 19:45 ` [PATCH v4 " SZEDER Gábor
2019-04-12 19:45 ` [PATCH v4 1/4] progress: make display_progress() return void SZEDER Gábor
2019-04-12 19:45 ` [PATCH v4 2/4] progress: assemble percentage and counters in a strbuf before printing SZEDER Gábor
2019-04-12 19:45 ` [PATCH v4 3/4] progress: clear previous progress update dynamically SZEDER Gábor
2019-04-12 19:45 ` [PATCH v4 4/4] progress: break too long progress bar lines SZEDER Gábor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190401141542.GJ32732@szeder.dev \
--to=szeder.dev@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=luke@mewburn.net \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).