[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH] support for gzipped transfer in wget-1.14
From: |
Ángel González |
Subject: |
Re: [Bug-wget] [PATCH] support for gzipped transfer in wget-1.14 |
Date: |
Sat, 07 Sep 2013 23:43:12 +0200 |
User-agent: |
Thunderbird |
On 06/09/13 21:21, Deepak Nagaraj wrote:
Hi all,
I found that GNU wget had no support for compressed file transfer. I have
modified the code to:
- send "Accept-Encoding: gzip" header
- check if response is gzipped, and if so, decompress it at end of download
- disable all related logic if --without-gzip is specified during configure
Good.
- not send the gzip header if a new option "-z" is specified
I'm not convinced about this. I would expect -z to *enable* zipping, not to
disable it. Given how unlikely it is to not desire gzipping, I would leave it a
long-option only.
I would even make it longer by not compressing "accept-encoding" into "ae".
You have a seemingly unrelated change to m4/wget.m4 It's ok, but please explain
why it was needed (and it should be added as a different change).
I would prefer not to download the file and then decompress, but to decompress
it on-the-fly. Also, the rename will fail on windows with EEXIST. If you feel
lazy to use zlib directly, you could spawn gzip -d and filter the file through
it. I foresee some problems when continuing a download, but I think there would
be some with your patch already.
It will probably be completely rewritten, but some comments about
decompress_file:
- tmpf uses a template of "fnXXXXXX", something like "wgetXXXXXX" would have
been more appropiate.
- You are leaking the temporary file in the several error cases.
- The error message says fwrite but you called write() (I suspect you wanted to
use stdio around the descriptor)
Finally one more consideration, what happens downloading a file with gzip
encoding if I had used -O - ? :)
Thanks for going ahead with this old request.