Hi Hubert,
Hubert Tarasiuk <hubert.taras...@gmail.com> writes:
> I have identified a potential drawback with the function
> `establish_connection`.
>
> [Patch #3]
> On error, it would free the `req` variable, but it never zeroed
> `*req_ref`. As the matter of fact, it only wrote to `req_ref` on
> successful exit (when it did not actually change).
> I suggest that this function never frees the `req` variable, because it
> never allocates it. (As opposed to `connreq`.)
> Instead, the caller (`gethttp`) releases the `req` when error occured.
>
> [Patch #4]
> My second idea is to change semantics of `resp_free` and `request_free`,
> so that they are similar to `xfree`, i.e.:
> 1) it is safe to call them with a NULL pointer
> 2) they ensure that the pointer is set to NULL after the call
> In order to achieve (2), a pointer to a pointer has to be passed.
> (Please note, that this patch depends on previous.)
thanks for your patches, I agree with you and the code looks nicer now.
I am going to push them tomorrow if nobody complains before (with the
following fix amended to your last patch).
diff --git a/src/http.c b/src/http.c
index 5338d20..9994d13 100644
--- a/src/http.c
+++ b/src/http.c
@@ -2506,7 +2506,7 @@ gethttp (struct url *u, struct http_stat *hs, int *dt,
struct url *proxy,
&using_ssl, inhibit_keep_alive, &sock);
if (err != RETROK)
{
- request_free (req);
+ request_free (&req);
return err;
}
}
Thanks for your contribution!
Giuseppe
