Re: [Bug-wget] gethttp cleanup

[Giuseppe Scrivano]

Hi Hubert,

Hubert Tarasiuk <address@hidden> 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