bug-wget
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-wget] gethttp cleanup


From: Hubert Tarasiuk
Subject: Re: [Bug-wget] gethttp cleanup
Date: Sun, 29 Mar 2015 11:56:54 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

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 am attaching all 4 of my patches against origin, because
(unfortunately), patch 4 depends on patch 2 and 3.

Have a good day.

-- 
Hubert Tarasiuk

Attachment: 0001-Factor-out-set_content_type-function-from-gethttp.patch
Description: Text Data

Attachment: 0002-Transform-read_header-label-and-goto-into-a-loop.patch
Description: Text Data

Attachment: 0003-Do-not-free-request-in-establish_connection-do-it-in.patch
Description: Text Data

Attachment: 0004-Change-semantics-of-resp_free-and-request_free-in-ht.patch
Description: Text Data

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]