CRC not verified when segmented downloading finished

[LoRenZo]

When the segmented download has been finished, the "CRC Checked" column in "Finished Downloads" tab displays "No". The Simple File Verification (.sfv) has been downloaded before. (The "enable automatic SFV checking" is turned on.) I'm using v0.707. (If it's because of the segmented downloading, then maybe that column should or could be removed, because it's useless in that case.)
By the way, why is a column called "CRC Checked" in the "Finished Uploads" tab too? What purpose has it?

[eMTee (realprogger)]

Seems CRC does not checked at all, tested either with segmented downloads disabled.

[eMTee (realprogger)]

The absence of the sfv check is because the code hasn't changed to work with the segmented download method (even if the changelog sates so for version 0.702). The attached patch can solve the problem...

Regarding the CRC check itself I can't see any reason to retrying the download once more if the sfv check is fails. I assume it works like that because in pre-TTH era it was a chance to get CRC errors because of a file transfer failure. But nowadays with TTH and zlib transfers its impossible so if there's no other reason to keep this behaviour I'd suggest to remove the CRC warn flag from sources and mark the source bad immediately if the sfv check fails.

[Jacek Sieka (arnetheduck)]

The queue looks like the correct place for the crc check now, but according to your own logic, the whole file should be flagged as invalid, not just the source (any file with tth checked will be bad)...

There is of course a slight chance that something happens with a file while it's being saved to disk - maybe calculate tth along with crc and if tth is ok and crc not flag the file as bad, if tth mismatches repair / redownload file (we had some repair code somewhere, didn't we?)...

[eMTee (realprogger)]

This is a more advanced version of the earlier patch, still doesn't remove all sources or repairs. The only change is to show the crc checked state correctly in the Finished window as well.
As for the check & repair, rechecker could be used for both with an addition that if its a check after a crc failure, it shouldn't move the unfinished file, even if it looks completly ok by TTH. I am not sure how to use it though in its current state. Maybe it would be better to do a TTH check in another way after a failed crc check in the same thread and if the TTH check fails then the item could be scheduled to a recheck....

[eMTee (realprogger)]

Here's the patch that removes all sources in case of a bad crc32 detected. It makes the sfv check function usable again, still does not cope with the case when the disk write fails (added as todo comment).
I think its worth to add now as is, because its a bit amusig that an option listed in the settings hasn't been working at all for almost 3 years.

[poy (poy)]

instead of removing the sources of a file, why not fully remove that file download itself? or at least, mark it as paused, and move it to the side. if the downloaded file does not have the right CRC, no amount of re-downloading from alternate sources will solve it; the user will have to go search for another file that fits.

[eMTee (realprogger)]

Somehow the user must know why a crc failed file wasn't downloaded. It shows the error in the queue item so its not a good idea to remove it automatically. I agree on that it can be set to paused to avoid further downloads from additional sources found by TTH.
What do you mean by 'move it to the side'?

[poy (poy)]

i meant an imaginary repository of unwanted TTHs... setting as "paused" is good enough though.
what happens if a user finds another file with the same name, different TTH (and likely, correct CRC)? will DC++ let the new download replace the old one?

[eMTee (realprogger)]

No. The user should remove the errorous items before beeing able to add ones with different TTH.

[eMTee (realprogger)]

Now it pauses the queue item and also, according to the new logic, also removed the CRC_WARN flag used for one more redownload try of a failed item (in the pre-TTH era).

In the future, adding a 'Failed' queue item status could solve both the redownloading and replace problem.

[eMTee (realprogger)]

Some cleaning and fix an issue with false error message.

[poy (poy)]

this patch looks ok barring the flags in the Download class are rebased (1<<1, 1<<2, 1<<3, etc).

[eMTee (realprogger)]

you mean

  enum {
   FLAG_ZDOWNLOAD = 1 << 1,
- FLAG_CALC_CRC32 = 1 << 2,
- FLAG_CRC32_OK = 1 << 3,
- FLAG_TREE_TRIED = 1 << 5,
- FLAG_TTH_CHECK = 1 << 6,
- FLAG_XML_BZ_LIST = 1 << 7
+ FLAG_TREE_TRIED = 1 << 2,
+ FLAG_TTH_CHECK = 1 << 3,
+ FLAG_XML_BZ_LIST = 1 << 4
  };

?

[poy (poy)]

yes, that's what i meant.

[poy (poy)]

Fixed in DC++ 0.780.

[LoRenZo (lorenzo-mailbox-deactivatedaccount)]

Thank you, it really works now, however, I did some tests, and I would like to notify that if verification ends with CRC inconsistency, the system log will display something like the following:

[23:36] vector::_M_range_insert<C:\Downloads\file.rar>

(The transferring window shows the following in such cases: CRC32 inconsistency (SFV-Check) )

I suppose another notification should be there, something like this:

[23:36] Download failed due to CRC inconsistency: C:\Downloads\file.rar

Note: I'm currently using DC++ 0.781.

[eMTee (realprogger)]

Yeah, something went very wrong with the log message addition, I can't see how... it'll be fixed in the next version :)