Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#4037 closed Bug (fixed)

[PATCH] timeChecked for the last piece is not saved to a resume file

Reported by: jusid Owned by: jordan
Priority: Normal Milestone: 2.22
Component: libtransmission Version: 2.20
Severity: Minor Keywords:
Cc:

Description

The attached patch fixes the following:

  • timeChecked for the last piece is not saved to a resume file.
  • timeChecked for the last piece is not loaded from a resume file.
  • if a torrent has some pieces with timeChecked=0 and some with timeChecked!=0, detailed information about that is not stored in a resume file. The "none checked" path is used in the code.
  • pieces with timeChecked=0 are not properly restored from a resume file. timeChecked is not 0 after the restore.

Attachments (2)

resume.diff (1.8 KB) - added by jusid 11 years ago.
patch
resume.2.diff (1.5 KB) - added by jordan 11 years ago.

Download all attachments as: .zip

Change History (8)

Changed 11 years ago by jusid

patch

comment:1 Changed 11 years ago by jordan

  • timeChecked for the last piece is not saved to a resume file.
  • timeChecked for the last piece is not loaded from a resume file.

Yes, you're right. Thanks for correcting this. Typical off-by-one stupidity. :)

Since the variables are named "end" rather than "last", they indicate the typical C idiom of being < rather than <=. We could rename the variables to fit the <= used in your patch, or we could add a "+1" when initializing the "end" variables...

  • if a torrent has some pieces with timeChecked=0 and some with timeChecked!=0, detailed information about that is not stored in a resume file. The "none checked" path is used in the code.

I don't think this is a problem. The important question in those two lines are whether or not all of the pieces' timestamps are old enough to require a recheck. If they're all too old, it doesn't matter how much too old they are.

  • pieces with timeChecked=0 are not properly restored from a resume file. timeChecked is not 0 after the restore.

Thank you. I really appreciate having extra eyes go through this code :)

Attached is a revised patch for your review.

Last edited 11 years ago by jordan (previous) (diff)

Changed 11 years ago by jordan

comment:2 Changed 11 years ago by jusid

The revised patch looks good. Thanks.

As for the "none checked" path. Simply I did some experiments with async piece verification during download, and timeChecked=0 was the important state, that should properly saved/restored. It is not so important for the current implementation and can be left as is...

comment:3 Changed 11 years ago by jordan

  • Keywords backport-2.1x added
  • Milestone changed from None Set to 2.30
  • Status changed from new to assigned

Fixed in r11974

comment:4 Changed 11 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:5 Changed 11 years ago by jordan

  • Keywords backport-2.2x added; backport-2.1x removed
  • Version changed from 2.21 to 2.20

comment:6 Changed 11 years ago by jordan

  • Keywords backport-2.2x removed
  • Milestone changed from 2.30 to 2.22

backported to 2.2x by r12026

Note: See TracTickets for help on using tickets.