Re: [Bug-wget] [Bug-Wget] Issues with Metalink support

[Darshit Shah]

On Sun, May 11, 2014 at 11:28 PM, Ángel González <address@hidden> wrote:
> On 07/05/14 23:46, Jure Grabnar wrote:
>>
>> Hello,
>>
>> I wrote two patches regarding this issue based on your suggestions.
>>
>> The 1st one is crucial for retrieve_from_file() function: it fixes 2
>> memory corruption bugs.
>>
>> The 2nd patch is more of an experiment than a real deal and is
>> intended to get some feedback. It changes parallel download to one
>> temporary file which is located in the selected directory.
>>
>> Before download starts it uses posix_fallocate() to allocate space and
>> mkstemp() for unique name. After download is completed it uses rename()
>> to rename temporary file to the final file name.
>>
>> After posix_fallocate() each thread opens file with fopen("wb").
>
> You could use w+b, even though you're not going to read from it.
>
>
>> I opted for fopen() rather than messing around with file descriptors
>> because I believe it's more portable. I don't know how Windows
>> would react to file descriptors and I don't have a proper Windows system
>> to test it out.
>
> It works fine.
> On Windows, FILE* are a layer on top of fds, which are themselves a layer
> over HANDLEs. To
> make things more complex, gnulib provides a different abstraction to wget.
> But it should work. The only special bit would be the need to add O_BINARY,
> which
> gnulib should already be doing for you.
>
>
>
>> Now, fopen("wb") means file, which was fallocate'd, is
>> truncated to zero but after first request from the thread, which is
>> reponsible for the last chunk, it would grow back to at least file_size
>> - chunk_size. I'm also not sure how devastating that is.
>
> It's up to the filesystem, but I think it would be better to do open (or
> dup) + fdopen()
> + fseek rather than the fopen(, "wb"); It also allows you to dispense with
> the barrier.
>
>
>
>> I'm attaching a handmade Metalink file which downloads a 50MB file for
>> testing purposes. Currently all threads connect to the same server and I
>> understand we don't support such behaviour but I guess 2-3 threads for
>> testing purpose don't hurt anyone. :)
>>
Does anyone have any objections to the above patches? Else we can merge them.
>> I'm open for suggestions.
>>
>> Regards,
>>
>> Jure Grabnar
>
>
>



-- 
Thanking You,
Darshit Shah