All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwen@google.com>,
	Han-Wen Nienhuys <hanwenn@gmail.com>
Subject: Re: [PATCH v2 4/7] reftable: avoid writing empty keys at the block layer
Date: Thu, 17 Feb 2022 15:55:09 -0800	[thread overview]
Message-ID: <xmqqee4159r6.fsf@gitster.g> (raw)
In-Reply-To: <ba036ee8543b2dc28ac046eb0c8c0aef9e751c80.1645106124.git.gitgitgadget@gmail.com> (Han-Wen Nienhuys via GitGitGadget's message of "Thu, 17 Feb 2022 13:55:21 +0000")

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
>  	int is_restart = 0;
>  	struct strbuf key = STRBUF_INIT;
>  	int n = 0;
> +	int err = -1;
>  
>  	reftable_record_key(rec, &key);
> +	if (!key.len) {
> +		err = REFTABLE_API_ERROR;
> +		goto done;
> +	}

OK; we get an API_ERROR when trying to write a bad one.  And ...

> @@ -332,6 +334,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)
>  	if (n < 0)
>  		return -1;
>  
> +	if (!key.len)
> +		return REFTABLE_FORMAT_ERROR;

... we get a FORMAT_ERROR when the data we try to read is bad
(i.e. not our fault).  OK.

> @@ -358,6 +363,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)
>  	int n = reftable_decode_key(key, &extra, empty, in);
>  	if (n < 0)
>  		return n;
> +	if (!key->len)
> +		return -1;

It is curious that this gets a different error out of the same
sequence, i.e. decode-key did not return an error but the length of
the key happens to be 0, not FORMAT_ERROR.

> diff --git a/reftable/writer.c b/reftable/writer.c
> index 944c2329ab5..d54215a50dc 100644
> --- a/reftable/writer.c
> +++ b/reftable/writer.c
> @@ -240,14 +240,13 @@ static int writer_add_record(struct reftable_writer *w,
>  
>  	writer_reinit_block_writer(w, reftable_record_type(rec));
>  	err = block_writer_add(w->block_writer, rec);
> -	if (err < 0) {
> +	if (err == -1) {
>  		/* we are writing into memory, so an error can only mean it
>  		 * doesn't fit. */
>  		err = REFTABLE_ENTRY_TOO_BIG_ERROR;
>  		goto done;
>  	}
>  
> -	err = 0;

Is this "doesn't fit" related to "we catch 0-length keys", or an
unrelated fix was included in this step by "rebase -i" mistake?


  reply	other threads:[~2022-02-17 23:55 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-12 18:07 [PATCH 0/7] reftable: avoid reading and writing empty keys Han-Wen Nienhuys via GitGitGadget
2022-01-12 18:07 ` [PATCH 1/7] Documentation: object_id_len goes up to 31 Han-Wen Nienhuys via GitGitGadget
2022-01-12 18:07 ` [PATCH 2/7] reftable: reject 0 object_id_len Han-Wen Nienhuys via GitGitGadget
2022-01-12 18:07 ` [PATCH 3/7] reftable: add a test that verifies that writing empty keys fails Han-Wen Nienhuys via GitGitGadget
2022-01-12 18:07 ` [PATCH 4/7] reftable: avoid writing empty keys at the block layer Han-Wen Nienhuys via GitGitGadget
2022-01-14  1:26   ` Junio C Hamano
2022-01-17 13:10     ` Han-Wen Nienhuys
2022-01-17 19:11       ` Junio C Hamano
2022-01-12 18:07 ` [PATCH 5/7] reftable: ensure that obj_id_len is >= 2 on writing Han-Wen Nienhuys via GitGitGadget
2022-01-12 18:07 ` [PATCH 6/7] reftable: add test for length of disambiguating prefix Han-Wen Nienhuys via GitGitGadget
2022-01-12 18:07 ` [PATCH 7/7] reftable: rename writer_stats to reftable_writer_stats Han-Wen Nienhuys via GitGitGadget
2022-02-17 13:55 ` [PATCH v2 0/7] reftable: avoid reading and writing empty keys Han-Wen Nienhuys via GitGitGadget
2022-02-17 13:55   ` [PATCH v2 1/7] Documentation: object_id_len goes up to 31 Han-Wen Nienhuys via GitGitGadget
2022-02-17 13:55   ` [PATCH v2 2/7] reftable: reject 0 object_id_len Han-Wen Nienhuys via GitGitGadget
2022-02-18  0:32     ` Junio C Hamano
2022-02-17 13:55   ` [PATCH v2 3/7] reftable: add a test that verifies that writing empty keys fails Han-Wen Nienhuys via GitGitGadget
2022-02-17 13:55   ` [PATCH v2 4/7] reftable: avoid writing empty keys at the block layer Han-Wen Nienhuys via GitGitGadget
2022-02-17 23:55     ` Junio C Hamano [this message]
2022-02-21 14:32       ` Han-Wen Nienhuys
2022-02-17 13:55   ` [PATCH v2 5/7] reftable: ensure that obj_id_len is >= 2 on writing Han-Wen Nienhuys via GitGitGadget
2022-02-18  0:01     ` Junio C Hamano
2022-02-17 13:55   ` [PATCH v2 6/7] reftable: add test for length of disambiguating prefix Han-Wen Nienhuys via GitGitGadget
2022-02-17 13:55   ` [PATCH v2 7/7] reftable: rename writer_stats to reftable_writer_stats Han-Wen Nienhuys via GitGitGadget
2022-02-18  0:02   ` [PATCH v2 0/7] reftable: avoid reading and writing empty keys Junio C Hamano
2022-02-21 18:46   ` [PATCH v3 " Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 1/7] Documentation: object_id_len goes up to 31 Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 2/7] reftable: reject 0 object_id_len Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 3/7] reftable: add a test that verifies that writing empty keys fails Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 4/7] reftable: avoid writing empty keys at the block layer Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 5/7] reftable: ensure that obj_id_len is >= 2 on writing Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 6/7] reftable: add test for length of disambiguating prefix Han-Wen Nienhuys via GitGitGadget
2022-02-21 18:46     ` [PATCH v3 7/7] reftable: rename writer_stats to reftable_writer_stats Han-Wen Nienhuys via GitGitGadget
2022-02-23 21:37     ` [PATCH v3 0/7] reftable: avoid reading and writing empty keys Junio C Hamano

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=xmqqee4159r6.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.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.