[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH] Re: [RFE / project idea]: convert-links for "tran
From: |
Ander Juaristi |
Subject: |
Re: [Bug-wget] [PATCH] Re: [RFE / project idea]: convert-links for "transparent proxy" mode |
Date: |
Tue, 29 Sep 2015 19:52:08 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
Hi Gabriel,
Thanks for your helpful comments.
I've corrected the patch.
For me, the most embarrassing thing was having forgotten the commit description
:D
On 09/24/2015 04:13 PM, Gabriel L. Somlo wrote:
Hi AJ,
Thanks for implementing this! I tested it, and the functionality
appears correct.
I've added a more detailed review inline, below.
+static char *
+convert_basename (const char *p, const struct urlpos *link)
+{
+ int len = link->size;
+ char *url = NULL;
+ char *org_basename = NULL, *local_basename = NULL;
+ char *result = url;
None of the string variables above really need to be initialized,
since you're going to assign them unconditionally below regardless.
I always initialize local variables by default. For me it's good practice.
Consider inverting the test. If basenames are *equal*, return 'url'
immediately, and save an unnecessary xstrdup() + xfree().
Otherwise, call uri_merge() and xfree(url) before returning the
result.
Done.
Finally, I also updated the documentation at doc/wget.texi.
Thanks much,
--Gabriel
Regards,
- AJ
0001-Added-convert-file-only-option.patch
Description: Text Data
- Re: [Bug-wget] [PATCH] Re: [RFE / project idea]: convert-links for "transparent proxy" mode,
Ander Juaristi <=