[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] gethttp cleanup
From: |
Giuseppe Scrivano |
Subject: |
Re: [Bug-wget] gethttp cleanup |
Date: |
Wed, 01 Apr 2015 17:01:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) |
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
- Re: [Bug-wget] gethttp cleanup,
Giuseppe Scrivano <=