File error when writing a piece of a torrent to disk is always reported as on tor->downloadDir

[boise]

...but if tor->incompleteDir is defined, the error probably (always?) actually occurred there.

I found this bug in Transmision-GTK 2.84, but since it is located in libtransmission, I guess it might affect other components as well.

In the readOrWritePiece() function in libtransmission/inout.c, when the call to the readOrWriteBytes() function returns an error code, tr_torrentSetLocalError() is called to register the error.

The problem lies in the parameters that are passed in that call to torrentSetLocalError(). The error code is correctly translated to a string with tr_strerror(err), but the value of the "path" variable is wrong.

The "path" variable is created on the line above (line 227 in v2.84) through a call to tr_buildPath(), and the bug is that the first argument in that call (the path to the file for which the error occurred) is incorrect. More specifically, the parameter is always passed as tor->downloadDir, but if tor->incompleteDir is defined, it is more likely (even certain?) that the error actually occurred there instead.

I would recommend that the full path to where the file error actually occurred is stored away when it occurs (for example by expanding the tr_torrent struct with something like a pathWhereFileErrorOccured field) to be reported in the error message.

[x190]

One could simply change "tor->downloadDir" to "tor->currentDir" in Line 227 of inout.c, however, even more accurate info can be given by doing the following.

    static int
    readOrWritePiece (tr_torrent       * tor,
                      int                ioMode,
                      tr_piece_index_t   pieceIndex,
                      uint32_t           pieceOffset,
                      uint8_t          * buf,
                      size_t             buflen)
    {
      int err = 0;
      tr_file_index_t fileIndex;
      uint64_t fileOffset;
      const tr_info * info = &tor->info;

      if (pieceIndex >= tor->info.pieceCount)
        return EINVAL;

      tr_ioFindFileLocation (tor, pieceIndex, pieceOffset,
                             &fileIndex, &fileOffset);

      while (buflen && !err)
        {
          const tr_file * file = &info->files[fileIndex];
          const uint64_t bytesThisPass = MIN (buflen, file->length - fileOffset);

          err = readOrWriteBytes (tor->session, tor, ioMode, fileIndex, fileOffset, buf, bytesThisPass);

          if ((err != 0) && (ioMode == TR_IO_WRITE) && (tor->error != TR_STAT_LOCAL_ERROR))
            {
              char * curdir = tr_torrentGetCurrentDir (tor);
              char * subpath = tr_sessionIsIncompleteFileNamingEnabled (tor->session)
                                       ? tr_torrentBuildPartial (tor, fileIndex)
                                       : tr_strdup (file->name);
              char * path = tr_buildPath (curdir, subpath, NULL);
              tr_torrentSetLocalError (tor, "%s (%s)", tr_strerror (err), path);

              tr_free (curdir);
              tr_free (subpath);
              tr_free (path);
            }

          buf += bytesThisPass;
          buflen -= bytesThisPass;
          fileIndex++;
          fileOffset = 0;
        }

      return err;
    }

[cfpp2p]

This ticket spawned from forum post ​https://forum.transmissionbt.com/viewtopic.php?f=2&t=16422

I might suggest to be exactly accurate as to where the write error occurred as reported by tr_torrentSetLocalError() that tr_torrentFindFile2() also be incorporated and move the detection to where the write error actually occurred. As implied by ​rb07 and tested implementation ​variation per ​patch

Although not as straight forward as x190's nice solution we could cover all the fringe cases if we wanted to.

[mike.dld]

Closed #6123 as duplicate of this ticket.