Re: [Bug-wget] gethttp cleanup

[Giuseppe Scrivano]

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.)
>
> Patch #4 would add some additional safety and clarity, but will probably
> cause slight run-time overhead (and the checks can be done manually when
> needed) - so let me know if you think that it is worth it.
>
> When these two issues are dealt with, a common cleanup code for
> `gethttp` will be easily possible for variables:
> - req
> - resp
> - head
> - message
> - type
>
> I will be thankful for your comments and suggestions.

I went ahead and pushed your patches!

Thanks again for your contribution.

Giuseppe