"Marco Costalba" [off-list ref] writes:
Patch to be applied above decompress helper series.
No way. That will mean that the resulting series will start
with a known bug.
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.
Pardon me. Are you talking about a bug you introduced earlier
in your own series that hasn't been applied (and you very well
know will not be until 1.5.4 is out, now we are deep in -rc
cycle)?
If so, you did a great disservice to me by sounding as if you
are blaming somebody else's existing bug. I wasted some time
hunting for a non-existent bug in the code that is being readied
for 1.5.4 final for quite some time, in order to pick only the
relevant "fix" from your patch.
It turns out, luckily, existing code did not have such a bug.
What a relief for the maintainer in bugfix-only freeze mode.
Next time around, please mark the patch on the Subject: line to
be squashed to your earlier [PATCH 5/6] before [PATCH 6/6].
That will also solve the bisectability problem.
Anyway, thanks. I was planning to queue the series in 'pu' or
'next' after tagging -rc3, so not be silent and giving a proper
fix was the right thing to do. My above rant is just about the
presentation.
On Jan 12, 2008 1:16 AM, Junio C Hamano [off-list ref] wrote:
Next time around, please mark the patch on the Subject: line to
be squashed to your earlier [PATCH 5/6] before [PATCH 6/6].
Very sorry for wasting your time I should have been more clear that it
was a bug in the new series. And of course this series is not to be
applied to stable git.
The only two points in the current code in master that I would like to
report to you are a _possible_ missing inflateEnd() before a new
inflateInit(), but I am not confident with that part of code to judge
if is a bug or not, anyway that's the _possible_ diff.
diff --git a/http-push.c b/http-push.c
index 55d0c94..e0a4cc6 100644
--- a/http-push.c
+++ b/http-push.c
@@ -307,6 +307,7 @@ static void start_fetch_loose(struct
transfer_request *request)
/* Reset inflate/SHA1 if there was an error reading the previous temp
file; also rewind to the beginning of the local file. */
if (prev_read == -1) {
+ inflateEnd(&request->stream);
memset(&request->stream, 0, sizeof(request->stream));
inflateInit(&request->stream);
SHA1_Init(&request->c);diff --git a/http-walker.c b/http-walker.c
index 2c37868..a18067c 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -182,6 +182,7 @@ static void start_object_request(struct walker *walker,
/* Reset inflate/SHA1 if there was an error reading the previous temp
file; also rewind to the beginning of the local file. */
if (prev_read == -1) {
+ inflateEnd(&obj_req->stream);
memset(&obj_req->stream, 0, sizeof(obj_req->stream));
inflateInit(&obj_req->stream);
SHA1_Init(&obj_req->c);
I have not created a proper patch becuase I don't know if the missing
inflateEnd(), it is a bug or not. The above diff it's just a way to
point you quickly and hopefully clearly to the interested code .
Sorry again for the trouble I had caused to you. For sure I will be
much more careful in the future to be clear in the subjects. And also
sorry for my rant but it was very late and I was tired after fighting
with that _my_ bug.
Marco