git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marco Costalba" <mcostalba@gmail.com>
To: "Junio C Hamano" <gitster@pobox.com>
Cc: "Git Mailing List" <git@vger.kernel.org>
Subject: [PATCH decompress BUG] Fix decompress_next_from() wrong argument value
Date: Fri, 11 Jan 2008 21:47:04 +0100	[thread overview]
Message-ID: <e5bfff550801111247l1ccf171ene5b53b8d6841a864@mail.gmail.com> (raw)

Function decompress_next_from() needs a pointer to a buffer
and the buffer size as arguments.

Interesting enough the function fill() that returns the
buffer pointer happens to modify also the buffer size,
stored in a variable at file scope.

So we need to guarantee fill() is called before to use buffer
size as argument in decompress_next_from()

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
Patch to be applied above decompress helper series.

Not to be pedantic, but have a function that gives two really
coupled values, as a buffer pointer and the size, the first as return
value and the second through a variable at file scope is not something
you are going to see advertised in the programming books!

Sorry for this little rant but this bug really made me crazy.

With this patch 'make test' runs with success!


 builtin-unpack-objects.c |    3 ++-
 index-pack.c             |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index f1a4883..72293ec 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -68,7 +68,8 @@ static void *get_data(unsigned long size)
 	decompress_into(&stream, buf, size);

 	for (;;) {
-		int ret = decompress_next_from(&stream, fill(1), len, Z_NO_FLUSH);
+		void* tmp = fill(1); // fill() modifies len, so be sure is evaluated as first
+		int ret = decompress_next_from(&stream, tmp, len, Z_NO_FLUSH);
 		use(len - stream.avail_in);
 		if (stream.total_out == size && ret == Z_STREAM_END)
 			break;
diff --git a/index-pack.c b/index-pack.c
index 30d7837..13b308d 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -173,7 +173,8 @@ static void *unpack_entry_data(unsigned long
offset, unsigned long size)
 	decompress_into(&stream, buf, size);

 	for (;;) {
-		int ret = decompress_next_from(&stream, fill(1), input_len, Z_NO_FLUSH);
+		void* tmp = fill(1); // fill() modifies input_len, so be sure is
evaluated as first
+		int ret = decompress_next_from(&stream, tmp, input_len, Z_NO_FLUSH);
 		use(input_len - stream.avail_in);
 		if (stream.total_out == size && ret == Z_STREAM_END)
 			break;
-- 
1.5.4.rc2.95.g0eaa-dirty

             reply	other threads:[~2008-01-11 20:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-11 20:47 Marco Costalba [this message]
2008-01-12  0:16 ` [PATCH decompress BUG] Fix decompress_next_from() wrong argument value Junio C Hamano
2008-01-12  7:06   ` Marco Costalba
2008-01-12  7:31     ` Junio C Hamano
2008-01-12  7:42       ` Marco Costalba
2008-01-12 18:44         ` 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=e5bfff550801111247l1ccf171ene5b53b8d6841a864@mail.gmail.com \
    --to=mcostalba@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 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 NNTP newsgroup(s).