Re: Rollback option patch

"Furi, Zoltan (Zoltan)" <[email protected]> writes:

> Please find attached my patch for the 'rollback' option. Please
> review and comment it.

You have the right idea, but there is a serious problem with the
patch: it doesn't do anything other than seek to a previous location
in the file.  The "rollback" functionality should also tell the remote
server to deliver the rolled-back bytes, along with the new ones.  For
example, if the file is 100K large and you have downloaded 50K and
specified a 10K rollback, Wget should request the last 60K (as opposed
to 50K) of contents.  Your patch does not do this.

Have you tested the change yourself?  If so, I'm surprised that it
worked for you, given the problems above.

Even regardless of the above show-stopper, there are several more
subtle points you should be aware of for possible future submissions:

* The "rollback" field should be of type wgint, not int, so rollback
  can work with large files.

* ftell is not large file aware; you have to use ftello instead.  This
  requires Autoconf checks for ftello and a non-portable
  implementation for Windows (which does have LFS, but not ftello).

* Does opening a file in append mode and then seeking to a previous
  location even work?  I believe fopen(..., "a") implies the O_APPEND
  mode to open(2), which means that *any* write to the file will
  append.  The solution is to either avoid "a" or ftruncate() the file
  (but both solutions have pitfalls).

* Before using fseek (and enable the rollback functionality in
  general), you must verify that you're writing to a plain file.
  Otherwise fseek() will silently (since you do no error checking)
  fail and the user will end up with corrupted file contents.  People
  *do* write things like `wget -qO- | ...'.

Note that the TODO list items are somewhat tricky: if they were easy
to do, they'd have been implemented by now!