All of lore.kernel.org
 help / color / mirror / Atom feed
From: Teng Long <dyroneteng@gmail.com>
To: avarab@gmail.com
Cc: derrickstolee@github.com, dyroneteng@gmail.com,
	git@vger.kernel.org, gitster@pobox.com, me@ttaylorr.com,
	tenglong.tl@alibaba-inc.com
Subject: Re: [PATCH v2 5/5] pack-bitmap.c: using error() instead of silently returning -1
Date: Fri,  6 May 2022 20:55:30 +0800	[thread overview]
Message-ID: <20220506125530.70989-1-dyroneteng@gmail.com> (raw)
In-Reply-To: <220421.86o80u77xj.gmgdl@evledraar.gmail.com>

On Thu, 21 Apr 2022 17:41:36 +0200, Ævar Arnfjörð Bjarmason wrote:

> First, I wondered if we were missing _(), but looking at other string in
> the file they're not using that already, looks like these all should,
> but we can fix this all up some other time in some i18n commit. It's
> fine to keep this for now.

Yes, I agree with you.

I also think I have a willingness to make another patchset to solve
the _() missing problems recently.

> But more importantly: I think this should be your 4/5. I.e. just make
> these an error() and you won't need to add e.g. this
> trace2_data_string() for a failed stat.

Make sense. Will adjust the order in next path.
 
> You will be inside your trace2 region, so any failure to stat etc. will
> be obvious from the structure of the data and the "error" event, no
> reason to have an additional trace2_data_string().

Yeah, I forgot about the "error()" already load the trace2 functions in.
Will remove the redundant trace2_data_string() where it's  obviously will
return error().

> Aside from that & as a general matter: Unless you have some use-case for
> trace2 data in this detail I'd think that it would be better just to
> skip logging it (except if we get it for free, such as with error()).
> 
> I.e. is this particular callsite really different from other places we
> fail to stat() or open() something?
> 
> It's all a moot point with the region + error, but just something to
> keep in mind.

Make sense.

And I think it's a good case in "open_midx_bitmap_1()" to add related
trace2_data_string() because there only a general error info in "cleanup:".


Thanks.

  reply	other threads:[~2022-05-06 12:56 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24 11:43 [PATCH v1 0/3] trace2 output for bitmap decision path Teng Long
2022-03-24 11:43 ` [PATCH v1 1/3] pack-bitmap.c: use "ret" in "open_midx_bitmap()" Teng Long
2022-03-24 19:11   ` Taylor Blau
2022-03-28  7:59     ` [PATCH v1 1/3] pack-bitmap.c: use "ret" in "open_midx_bitmap() Teng Long
2022-03-30  2:39       ` Taylor Blau
2022-03-24 11:44 ` [PATCH v1 2/3] pack-bitmap.c: add "break" statement in "open_pack_bitmap()" Teng Long
2022-03-24 18:40   ` Junio C Hamano
2022-03-24 19:06     ` Taylor Blau
2022-03-24 19:03   ` Taylor Blau
2022-03-29  2:49     ` Teng Long
2022-03-30  2:55       ` Taylor Blau
2022-03-30  7:32         ` Teng Long Teng Long
2022-03-30 13:17           ` Ævar Arnfjörð Bjarmason
2022-03-24 11:44 ` [PATCH v1 3/3] bitmap: add trace outputs during open "bitmap" file Teng Long
2022-03-24 18:42   ` Junio C Hamano
2022-03-24 13:22 ` [PATCH v1 0/3] trace2 output for bitmap decision path Ævar Arnfjörð Bjarmason
2022-03-29  7:38   ` Teng Long Teng Long
2022-03-29  8:54     ` Ævar Arnfjörð Bjarmason
2022-04-21 13:26 ` [PATCH v2 0/5] trace2 output for bitmap decision path Teng Long
2022-04-21 13:26   ` [PATCH v2 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-05-11 21:31     ` Taylor Blau
2022-04-21 13:26   ` [PATCH v2 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-05-11 21:31     ` Taylor Blau
2022-04-21 13:26   ` [PATCH v2 3/5] pack-bitmap.c: make warnings more detailed when opening bitmap Teng Long
2022-04-21 17:25     ` Taylor Blau
2022-05-06  9:08       ` Teng Long
2022-04-21 13:26   ` [PATCH v2 4/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-04-21 15:51     ` Ævar Arnfjörð Bjarmason
2022-05-06 11:27       ` Teng Long
2022-05-06 11:53       ` Teng Long
2022-04-21 16:32     ` Jeff Hostetler
2022-05-06 12:43       ` Teng Long
2022-05-10 20:47         ` Jeff Hostetler
2022-04-21 13:26   ` [PATCH v2 5/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-04-21 15:41     ` Ævar Arnfjörð Bjarmason
2022-05-06 12:55       ` Teng Long [this message]
2022-06-12  7:44   ` [PATCH v3 0/5] Teng Long
2022-06-12  7:44     ` Teng Long
2022-06-12  7:44       ` [PATCH v3 0/5] trace2 output for bitmap decision path Teng Long
2022-06-12  7:44       ` [PATCH v3 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-12  7:44       ` [PATCH v3 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-12  7:44       ` [PATCH v3 3/5] pack-bitmap.c: make warnings support i18N when opening bitmap Teng Long
2022-06-12  7:44       ` [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-14  1:15         ` Taylor Blau
2022-06-20 13:17           ` Teng Long
2022-06-12  7:44       ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-06-13 20:59         ` Junio C Hamano
2022-06-20 13:32           ` Teng Long
2022-06-14  1:40         ` Taylor Blau
2022-06-21  6:58           ` Teng Long
2022-06-22 12:51         ` Jeff Hostetler
2022-06-23  9:38           ` Teng Long
2022-06-23 15:14             ` Jeff Hostetler
2022-06-24  8:27               ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" Teng Long
2022-06-21 13:25       ` [PATCH v3 0/5] trace2 output for bitmap decision path Teng Long
2022-06-21 13:25         ` [PATCH v3 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-21 13:25         ` [PATCH v3 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-21 13:25         ` [PATCH v3 3/5] pack-bitmap.c: make warnings support i18N when opening bitmap Teng Long
2022-06-21 13:25         ` [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-21 13:25         ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-06-22 13:04           ` Jeff Hostetler
2022-06-22 15:12             ` Junio C Hamano
2022-06-28  8:17         ` [PATCH v5 0/5] tr2: avoid to print "interesting" config repeatedly Teng Long
2022-06-28  8:17           ` [PATCH v5 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-28  8:17           ` [PATCH v5 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-28  8:17           ` [PATCH v5 3/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-28 18:04             ` Junio C Hamano
2022-07-05  9:04               ` Teng Long
2022-07-05 18:23                 ` Junio C Hamano
2022-06-28  8:17           ` [PATCH v5 4/5] pack-bitmap.c: retrieve missing i18n translations Teng Long
2022-06-28  8:58             ` Ævar Arnfjörð Bjarmason
2022-06-28 17:28               ` Eric Sunshine
2022-07-06 14:19                 ` Teng Long
2022-07-06 14:06               ` Teng Long
2022-06-28 18:07             ` Junio C Hamano
2022-07-07 11:59               ` Teng Long
2022-07-07 16:45                 ` Junio C Hamano
2022-07-11 11:04                   ` Teng Long
2022-06-28  8:17           ` [PATCH v5 5/5] tr2: avoid to print "interesting" config repeatedly Teng Long
2022-06-28  9:13             ` Ævar Arnfjörð Bjarmason
2022-06-28 18:12               ` Junio C Hamano
2022-07-01 14:31                 ` Jeff Hostetler
2022-07-11  4:11                   ` Teng Long
2022-07-11  3:51               ` Teng Long
2022-07-11 12:43           ` [PATCH v6 0/7] trace2: dump scope when print "interesting" config Teng Long
2022-07-11 12:43             ` [PATCH v6 1/7] clean: fixed issues related to text output format Teng Long
2022-07-11 21:08               ` Junio C Hamano
2022-07-13 11:44                 ` Teng Long
2022-07-11 12:43             ` [PATCH v6 2/7] pack-bitmap.c: mark more strings for translations Teng Long
2022-07-11 12:43             ` [PATCH v6 3/7] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-07-11 12:44             ` [PATCH v6 4/7] pack-bitmap.c: don't ignore ENOENT silently Teng Long
2022-07-11 14:38               ` Ævar Arnfjörð Bjarmason
2022-07-13 14:14                 ` Teng Long
2022-07-11 21:22               ` Junio C Hamano
2022-07-14 15:25                 ` Teng Long
2022-07-11 12:44             ` [PATCH v6 5/7] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-07-11 14:53               ` Ævar Arnfjörð Bjarmason
2022-07-15  2:34                 ` Teng Long
2022-07-11 12:44             ` [PATCH v6 6/7] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-07-11 12:44             ` [PATCH v6 7/7] tr2: dump names if config exist in multiple scopes Teng Long
2022-07-11 14:40               ` Ævar Arnfjörð Bjarmason
2022-07-11 19:19               ` Jeff Hostetler
2022-07-11 14:59             ` [PATCH v6 0/7] trace2: dump scope when print "interesting" config Ævar Arnfjörð Bjarmason
2022-07-18  8:36               ` Teng Long
2022-07-18 16:45             ` [PATCH v7 " Teng Long
2022-07-18 16:46               ` [PATCH v7 1/7] pack-bitmap.c: fix formatting of error messages Teng Long
2022-07-18 16:46               ` [PATCH v7 2/7] pack-bitmap.c: mark more strings for translations Teng Long
2022-07-18 16:46               ` [PATCH v7 3/7] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-07-18 16:46               ` [PATCH v7 4/7] pack-bitmap.c: do not ignore error when opening a bitmap file Teng Long
2022-07-18 16:46               ` [PATCH v7 5/7] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-07-18 16:46               ` [PATCH v7 6/7] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-07-18 16:46               ` [PATCH v7 7/7] tr2: dump names if config exist in multiple scopes Teng Long
2022-07-18 20:13                 ` Jeff Hostetler
2022-07-19  7:40                   ` tenglong.tl
2022-07-19 21:03                 ` Junio C Hamano
2022-07-20 12:48                   ` tenglong.tl
2022-07-18 18:57               ` [PATCH v7 0/7] trace2: dump scope when print "interesting" config Junio C Hamano
2022-07-18 19:07                 ` Ævar Arnfjörð Bjarmason
2022-07-19 11:26                   ` tenglong.tl
2022-07-19 11:42                     ` Ævar Arnfjörð Bjarmason
2022-07-19 12:34                       ` tenglong.tl
2022-07-21  9:05               ` [PATCH v8 0/6] pack-bitmap.c: optimize error messages tenglong.tl
2022-07-21  9:05                 ` [PATCH v8 1/6] pack-bitmap.c: fix formatting of " tenglong.tl
2022-07-21  9:05                 ` [PATCH v8 2/6] pack-bitmap.c: mark more strings for translations tenglong.tl
2022-07-21  9:05                 ` [PATCH v8 3/6] pack-bitmap.c: rename "idx_name" to "bitmap_name" tenglong.tl
2022-07-21  9:05                 ` [PATCH v8 4/6] pack-bitmap.c: do not ignore error when opening a bitmap file tenglong.tl
2022-07-21  9:05                 ` [PATCH v8 5/6] pack-bitmap.c: using error() instead of silently returning -1 tenglong.tl
2022-07-21  9:05                 ` [PATCH v8 6/6] pack-bitmap.c: continue looping when first MIDX bitmap is found tenglong.tl
2022-07-21 23:01                 ` [PATCH v8 0/6] pack-bitmap.c: optimize error messages Junio C Hamano
2022-07-22  6:17                   ` tenglong.tl

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220506125530.70989-1-dyroneteng@gmail.com \
    --to=dyroneteng@gmail.com \
    --cc=avarab@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=tenglong.tl@alibaba-inc.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.