Changeset 3665

Timestamp:

Oct 31, 2007, 6:10:54 PM (15 years ago)

Author:

charles

Message:

"corruption" fix part 3: cleanup. remove unneeded possible points of failure.

Location:

branches/0.9x

Files:

• configure.ac (modified) (1 diff)
• libtransmission/fdlimit.c (modified) (22 diffs)
• libtransmission/inout.c (modified) (15 diffs)
• libtransmission/ipcparse.c (modified) (1 diff)
• libtransmission/transmission.h (modified) (1 diff)
• libtransmission/utils.c (modified) (1 diff)

branches/0.9x/configure.ac

@@ -30 +30 @@
 AC_HEADER_TIME
 AC_CHECK_FUNCS([lrintf strlcpy strlcat daemon dirname basename])
+AC_CHECK_SIZEOF([void*])
 AC_PROG_INSTALL
 AC_PROG_MAKE_SET

branches/0.9x/libtransmission/fdlimit.c

@@ -25 +25 @@
 #include <assert.h>
 #include <errno.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -43 +44 @@
 #include "transmission.h"
 #include "trcompat.h"
+#include "list.h"
 #include "net.h"
 #include "platform.h"
 #include "utils.h"
+
+#if SIZEOF_VOIDP==8
+#define TR_UINT_TO_PTR(i) (void*)((uint64_t)i)
+#else
+#define TR_UINT_TO_PTR(i) ((void*)((uint32_t)i))
+#endif
 
 /**
@@ -82 +90 @@
 enum
 {
-    TR_MAX_OPEN_FILES = 16, /* That is, real files, not sockets */
+    TR_MAX_OPEN_FILES = 16, /* real files, not sockets */
 
     TR_RESERVED_FDS   = 16 /* sockets reserved for tracker connections */
@@ -92 +100 @@
     unsigned int  isWritable : 1;
     char          filename[MAX_PATH_LENGTH];
-    int           file;
+    int           fd;
     uint64_t      date;
 };
@@ -118 +126 @@
 {
     struct tr_openfile * file = &gFd->open[i];
-    struct stat sb;
     char * dir;
     int flags;
@@ -131 +138 @@
     }
 
-    /* Make sure the parent folder exists */
-    if( stat( dir, &sb ) || !S_ISDIR( sb.st_mode ) ) {
-        free( dir );
-        return TR_ERROR_IO_PARENT;
-    }
-
-    errno = 0;
-    flags = 0;
+    /* open the file */
+    flags = write ? (O_RDWR | O_CREAT) : O_RDONLY;
 #ifdef WIN32
     flags |= O_BINARY;
 #endif
-    flags |= write ? (O_RDWR | O_CREAT) : O_RDONLY;
-    file->file = open( filename, flags, 0666 );
+    errno = 0;
+    file->fd = open( filename, flags, 0600 );
     free( dir );
-    if( write && ( file->file < 0 ) )
-    {
-        const int ret = tr_ioErrorFromErrno();
-        if( errno )
+    if( file->fd < 0 ) {
+        if( errno ) {
             tr_err( "Couldn't open '%s': %s", filename, strerror(errno) );
-        else
+            return tr_ioErrorFromErrno();
+        } else {
             tr_err( "Couldn't open '%s'", filename );
-        return ret;
+            return TR_ERROR_IO_OTHER;
+        }
     }
 
@@ -161 +162 @@
 fileIsOpen( const struct tr_openfile * o )
 {
-    return o->file >= 0;
+    return o->fd >= 0;
 }
 
@@ -167 +168 @@
 TrCloseFile( int i )
 {
-    struct tr_openfile * file = &gFd->open[i];
+    struct tr_openfile * o = &gFd->open[i];
 
     assert( i >= 0 );
     assert( i < TR_MAX_OPEN_FILES );
-    assert( fileIsOpen( file ) );
-
-    dbgmsg( "closing %s in slot %d writable %c",
-            file->filename, i, file->isWritable?'y':'n' );
-    close( file->file );
-    file->file = -1;
-    file->isCheckedOut = 0;
+    assert( fileIsOpen( o ) );
+
+    dbgmsg( "closing slot #%d, %s", i, o->filename );
+    close( o->fd );
+    o->fd = -1;
+    o->isCheckedOut = 0;
     tr_condSignal( gFd->cond );
 }
@@ -192 +192 @@
     int i, winner;
     struct tr_openfile * o;
+
+    assert( filename && *filename );
+    assert( write==0 || write==1 );
 
     dbgmsg( "looking for file '%s', writable %c", filename, write?'y':'n' );
@@ -211 +214 @@
             dbgmsg( "found it!  it's open, but checked out.  waiting..." );
             tr_condWait( gFd->cond, gFd->lock );
-            i = -1;
+            i = -1; /* reloop */
             continue;
         }
@@ -218 +221 @@
             dbgmsg( "found it!  it's open and available, but isn't writable. closing..." );
             TrCloseFile( i );
-            continue;
+            break;
         }
 
@@ -243 +246 @@
             }
 
-            if( o->date < date ) {
+            if( date > o->date ) {
+                date = o->date;
                 winner = i;
-                date = o->date;
             }
         }
@@ -272 +275 @@
 
         dbgmsg( "opened '%s' in slot %d, write %c", filename, winner, write?'y':'n' );
-        strlcpy( gFd->open[winner].filename, filename, MAX_PATH_LENGTH );
-        gFd->open[winner].isWritable = write;
+        strlcpy( o->filename, filename, sizeof( o->filename ) );
+        o->isWritable = write;
     }
 
     dbgmsg( "checking out '%s' in slot %d", filename, winner );
-    gFd->open[winner].isCheckedOut = 1;
-    gFd->open[winner].date = tr_date();
+    o->isCheckedOut = 1;
+    o->date = tr_date();
     tr_lockUnlock( gFd->lock );
-    return gFd->open[winner].file;
+    return o->fd;
 }
 
@@ -291 +294 @@
     for( i=0; i<TR_MAX_OPEN_FILES; ++i ) {
         struct tr_openfile * o = &gFd->open[i];
-        if( o->file == file ) {
+        if( o->fd == file ) {
             dbgmsg( "releasing file '%s' in slot #%d", o->filename, i );
             if( o->isWritable )
-                fsync( o->file ); /* fflush */
+                fsync( o->fd ); /* fflush */
             o->isCheckedOut = 0;
             break;
@@ -310 +313 @@
 ***/
 
-struct tr_socket
-{
-    int socket;
-    int priority;
-};
-
-/* Remember the priority of every socket we open, so that we can keep
- * track of how many reserved file descriptors we are using */
-static struct tr_socket * gSockets = NULL;
-static int gSocketsSize = 0;
-static int gSocketsCount = 0;
+static tr_list * reservedSockets = NULL;
 
 static void
-SocketSetPriority( int s, int priority )
-{
-    if( gSocketsSize <= gSocketsCount ) {
-        gSocketsSize += 64;
-        gSockets = tr_renew( struct tr_socket, gSockets, gSocketsSize );
-    }
-
-    gSockets[gSocketsCount].socket = s;
-    gSockets[gSocketsCount].priority = priority;
-    ++gSocketsCount;
+setSocketPriority( int fd, int isReserved )
+{
+    if( isReserved )
+        tr_list_append( &reservedSockets, TR_UINT_TO_PTR(fd) );
 }
 
 static int
-SocketGetPriority( int s )
-{
-    int i, ret;
-
-    for( i=0; i<gSocketsCount; ++i )
-        if( gSockets[i].socket == s )
-            break;
-
-    if( i >= gSocketsCount ) {
-        tr_err( "could not find that socket (%d)!", s );
-        return -1;
-    }
-
-    ret = gSockets[i].priority;
-    gSocketsCount--;
-    memmove( &gSockets[i], &gSockets[i+1],
-            ( gSocketsCount - i ) * sizeof( struct tr_socket ) );
-    return ret;
+socketWasReserved( int fd )
+{
+    return tr_list_remove_data( &reservedSockets, TR_UINT_TO_PTR(fd) ) != NULL;
 }
 
@@ -360 +332 @@
 {
     int s = -1;
-
     tr_lockLock( gFd->lock );
 
@@ -367 +338 @@
 
     if( priority || ( gFd->normal < gFd->normalMax ) )
-       if( ( s = socket( AF_INET, type, 0 ) ) < 0 )
-           tr_err( "Couldn't create socket (%s)", strerror( sockerrno ) );
+        if( ( s = socket( AF_INET, type, 0 ) ) < 0 )
+            tr_err( "Couldn't create socket (%s)", strerror( sockerrno ) );
 
     if( s > -1 )
     {
-        SocketSetPriority( s, priority );
+        setSocketPriority( s, priority );
+
         if( priority )
-            gFd->reserved++;
+            ++gFd->reserved;
         else
-            gFd->normal++;
-    }
+            ++gFd->normal;
+    }
+
+    assert( gFd->reserved >= 0 );
+    assert( gFd->normal >= 0 );
+
     tr_lockUnlock( gFd->lock );
-
     return s;
 }
@@ -387 +362 @@
 {
     int s = -1;
-    unsigned len;
+    unsigned int len;
     struct sockaddr_in sock;
 
@@ -401 +376 @@
     if( s > -1 )
     {
-        SocketSetPriority( s, 0 );
+        setSocketPriority( s, 0 );
         *addr = sock.sin_addr;
         *port = sock.sin_port;
@@ -424 +399 @@
 tr_fdSocketClose( int s )
 {
-    if( s >= 0 )
-    {
-        tr_lockLock( gFd->lock );
+    tr_lockLock( gFd->lock );
+
+    if( s >= 0 ) {
         socketClose( s );
-        if( SocketGetPriority( s ) )
-            gFd->reserved--;
+        if( socketWasReserved( s ) )
+            --gFd->reserved;
         else
-            gFd->normal--;
-        tr_lockUnlock( gFd->lock );
-    }
+            --gFd->normal;
+    }
+
+    assert( gFd->reserved >= 0 );
+    assert( gFd->normal >= 0 );
+
+    tr_lockUnlock( gFd->lock );
 }
 
@@ -465 +444 @@
 
     for( i=0; i<TR_MAX_OPEN_FILES; ++i )
-        gFd->open[i].file = -1;
+        gFd->open[i].fd = -1;
           
 }
@@ -481 +460 @@
     tr_condFree( gFd->cond );
 
-    tr_free( gSockets );
+    tr_list_free( &reservedSockets, NULL );
     tr_free( gFd );
 }

branches/0.9x/libtransmission/inout.c

@@ -47 +47 @@
 
 static int
-readOrWriteBytes ( const tr_torrent    * tor,
-                   int                   ioMode,
-                   int                   fileIndex,
-                   uint64_t              fileOffset,
-                   void                * buf,
-                   size_t                buflen )
+readOrWriteBytes( const tr_torrent  * tor,
+                  int                 ioMode,
+                  int                 fileIndex,
+                  uint64_t            fileOffset,
+                  void              * buf,
+                  size_t              buflen )
 {
     const tr_info * info = &tor->info;
@@ -71 +71 @@
     if( !file->length )
         return 0;
-    else if ((ioMode==TR_IO_READ) && stat( path, &sb ) ) /* fast check to make sure file exists */
+    else if ((ioMode==TR_IO_READ) && stat( path, &sb ) ) /* does file exist? */
         ret = tr_ioErrorFromErrno ();
     else if ((fd = tr_fdFileOpen ( path, ioMode==TR_IO_WRITE )) < 0)
@@ -89 +89 @@
 
 static void
-findFileLocation ( const tr_torrent * tor,
-                   int                  pieceIndex,
-                   int                  pieceOffset,
-                   int                * fileIndex,
-                   uint64_t           * fileOffset )
+findFileLocation( const tr_torrent * tor,
+                  int                  pieceIndex,
+                  int                  pieceOffset,
+                  int                * fileIndex,
+                  uint64_t           * fileOffset )
 {
     const tr_info * info = &tor->info;
@@ -117 +117 @@
 #ifdef WIN32
 static int
-ensureMinimumFileSize ( const tr_torrent  * tor,
-                        int                 fileIndex,
-                        uint64_t            minSize ) /* in bytes */
+ensureMinimumFileSize( const tr_torrent  * tor,
+                       int                 fileIndex,
+                       uint64_t            minBytes )
 {
     int fd;
@@ -128 +128 @@
 
     assert ( 0<=fileIndex && fileIndex<tor->info.fileCount );
-    assert ( minSize <= file->length );
+    assert ( minBytes <= file->length );
 
     tr_buildPath ( path, sizeof(path), tor->destination, file->name, NULL );
@@ -137 +137 @@
     else if (fstat (fd, &sb) ) /* how big is the file? */
         ret = tr_ioErrorFromErrno ();
-    else if (sb.st_size >= (off_t)minSize) /* already big enough */
+    else if (sb.st_size >= (off_t)minBytes) /* already big enough */
         ret = TR_OK;
-    else if (!ftruncate( fd, minSize )) /* grow it */
+    else if (!ftruncate( fd, minBytes )) /* grow it */
         ret = TR_OK;
     else /* couldn't grow it */
@@ -152 +152 @@
 
 static int
-readOrWritePiece ( tr_torrent       * tor,
-                   int                ioMode,
-                   int                pieceIndex,
-                   int                pieceOffset,
-                   uint8_t          * buf,
-                   size_t             buflen )
+readOrWritePiece( tr_torrent  * tor,
+                  int           ioMode,
+                  int           pieceIndex,
+                  int           pieceOffset,
+                  uint8_t     * buf,
+                  size_t        buflen )
 {
     int ret = 0;
@@ -194 +194 @@
 tr_ioRead( tr_torrent * tor, int pieceIndex, int begin, int len, uint8_t * buf )
 {
-    return readOrWritePiece ( tor, TR_IO_READ, pieceIndex, begin, buf, len );
+    return readOrWritePiece( tor, TR_IO_READ, pieceIndex, begin, buf, len );
 }
 
@@ -200 +200 @@
 tr_ioWrite( tr_torrent * tor, int pieceIndex, int begin, int len, uint8_t * buf )
 {
-    return readOrWritePiece ( tor, TR_IO_WRITE, pieceIndex, begin, buf, len );
+    return readOrWritePiece( tor, TR_IO_WRITE, pieceIndex, begin, buf, len );
 }
 
@@ -208 +208 @@
 
 static int
-tr_ioRecalculateHash( tr_torrent    * tor,
-                      int             pieceIndex,
-                      uint8_t       * setme )
+tr_ioRecalculateHash( tr_torrent  * tor,
+                      int           pieceIndex,
+                      uint8_t     * setme )
 {
     int n;
@@ -237 +237 @@
 {
     uint8_t hash[SHA_DIGEST_LENGTH];
-    int ret = tr_ioRecalculateHash( tor, pieceIndex, hash )
-           || memcmp( hash, tor->info.pieces[pieceIndex].hash, SHA_DIGEST_LENGTH );
+    const int ret = tr_ioRecalculateHash( tor, pieceIndex, hash )
+        || memcmp( hash, tor->info.pieces[pieceIndex].hash, SHA_DIGEST_LENGTH );
     tr_dbg ("torrent [%s] piece %d hash check: %s",
             tor->info.name, pieceIndex, (ret?"FAILED":"OK"));
@@ -267 +267 @@
     }
 
-    tr_peerMgrSetBlame( tor->handle->peerMgr, tor->info.hash, pieceIndex, success );
+    tr_peerMgrSetBlame( tor->handle->peerMgr, tor->info.hash,
+                        pieceIndex, success );
 
     return ret;
@@ -290 +291 @@
 }
 
-struct recheck_node currentNode;
+static struct recheck_node currentNode;
 
 static tr_list * recheckList = NULL;
@@ -397 +398 @@
 compareRecheckByTorrent( const void * va, const void * vb )
 {
-    const struct recheck_node * a = ( const struct recheck_node * ) va;
-    const struct recheck_node * b = ( const struct recheck_node * ) vb;
-    return a->torrent - b->torrent;
+    const struct recheck_node * a = va;
+    const tr_torrent * b = vb;
+    return a->torrent - b;
 }
 
@@ -420 +421 @@
     else
     {
-        struct recheck_node tmp;
-        struct recheck_node * node;
-        tmp.torrent = tor;
-        node = tr_list_remove( &recheckList,
-                               &tmp,
-                               compareRecheckByTorrent );
-        tr_free( node );
+        tr_free( tr_list_remove( &recheckList, tor, compareRecheckByTorrent ) );
         tor->recheckState = TR_RECHECK_NONE;
     }

branches/0.9x/libtransmission/ipcparse.c

@@ -781 +781 @@
                     tr_bencInitStr( item, "assert", -1, 1 );
                 }
-                else if( TR_ERROR_ISSET( TR_ERROR_IO_PARENT, error ) )
-                {
-                    tr_bencInitStr( item, "io-parent", -1, 1 );
-                }
                 else if( TR_ERROR_ISSET( TR_ERROR_IO_PERMISSIONS, error ) )
                 {

branches/0.9x/libtransmission/transmission.h

@@ -81 +81 @@
 /* I/O errors */
 #define TR_ERROR_IO_MASK            0x000000FF
-#define TR_ERROR_IO_PARENT          0x80000001
 #define TR_ERROR_IO_PERMISSIONS     0x80000002
 #define TR_ERROR_IO_SPACE           0x80000004

branches/0.9x/libtransmission/utils.c

@@ -648 +648 @@
         case TR_ERROR_ASSERT:
             return "Assert error";
-        case TR_ERROR_IO_PARENT:
-            return "Download folder does not exist";
         case TR_ERROR_IO_PERMISSIONS:
             return "Insufficient permissions";