Fixed all compiler warnings. For debugging builds, also made

the compiler treat warnings as errors.

CMakeLists.txt

@@ -40,7 +40,7 @@ if (TRACE)
 endif()
 
 if (CMAKE_BUILD_TYPE STREQUAL Debug)
-    add_definitions(-DDEBUG)
+    add_definitions(-DDEBUG -Werror -Wall)
     set(S3_CONFIGURE_OPT --enable-debug)
 endif()
 

src/AppendTask.cpp

@@ -61,7 +61,7 @@ bool AppendTask::run()
         check_error("AppendTask read data", false);
         readCount += toRead;
         uint writePos = 0;
-        int err;
+
         while (writeCount < readCount)
         {
             try

src/Cache.cpp

@@ -186,11 +186,10 @@ void Cache::read(const vector<string> &keys)
     s.unlock();
     
     // start downloading the keys to fetch
-    int dl_err;
     vector<int> dl_errnos;
     vector<size_t> sizes;
     if (!keysToFetch.empty())
-        dl_err = downloader.download(keysToFetch, &dl_errnos, &sizes);
+        downloader.download(keysToFetch, &dl_errnos, &sizes);
     
     size_t sum_sizes = 0;
     for (size_t &size : sizes)
@@ -272,7 +271,7 @@ void Cache::exists(const vector<string> &keys, vector<bool> *out) const
 {
     out->resize(keys.size());
     boost::unique_lock<boost::mutex> s(lru_mutex);
-    for (int i = 0; i < keys.size(); i++)
+    for (uint i = 0; i < keys.size(); i++)
         (*out)[i] = (m_lru.find(keys[i]) != m_lru.end());
 }
 
@@ -370,7 +369,7 @@ void Cache::_makeSpace(size_t size)
             remove it from our structs
             update current size
         */
-        assert(currentCacheSize >= statbuf.st_size);
+        assert(currentCacheSize >= (size_t) statbuf.st_size);
         currentCacheSize -= statbuf.st_size;
         thisMuch -= statbuf.st_size;
         Synchronizer::get()->flushObject(*it);

src/CloudStorage.cpp

@@ -18,7 +18,7 @@ namespace
 string tolower(const string &s)
 {
     string ret(s);
-    for (int i = 0; i < ret.length(); i++)
+    for (uint i = 0; i < ret.length(); i++)
         ret[i] = ::tolower(ret[i]);
     return ret;
 }

src/Config.cpp

@@ -52,7 +52,7 @@ Config::Config() : die(false)
         paths.push_back(cs_install_dir);
     paths.push_back("/etc");
     
-    for (int i = 0; i < paths.size(); i++)
+    for (uint i = 0; i < paths.size(); i++)
     {
         if (boost::filesystem::exists(paths[i] + "/storagemanager.cnf"))
         {

src/CopyTask.cpp

@@ -65,7 +65,6 @@ bool CopyTask::run()
     }
     
     sm_response *resp = (sm_response *) buf;
-    uint payloadLen = 0;
     resp->returnCode = 0;
     success = write(*resp, 0);
     return success;

src/Downloader.cpp

@@ -4,6 +4,7 @@
 #include <string>
 #include <errno.h>
 #include <iostream>
+#include <boost/filesystem.hpp>
 
 using namespace std;
 namespace storagemanager
@@ -36,9 +37,8 @@ inline boost::mutex & Downloader::getDownloadMutex()
     return download_mutex;
 }
 
-int Downloader::download(const vector<const string *> &keys, vector<int> *errnos, vector<size_t> *sizes)
+void Downloader::download(const vector<const string *> &keys, vector<int> *errnos, vector<size_t> *sizes)
 {
-    //volatile uint counter = keys.size();
     uint counter = keys.size();
     boost::condition condvar;
     boost::mutex m;
@@ -96,19 +96,14 @@ int Downloader::download(const vector<const string *> &keys, vector<int> *errnos
     s.unlock();
             
     // check for errors & propagate
-    int ret = 0;
     errnos->resize(keys.size());
+    char buf[80];
     for (i = 0; i < keys.size(); i++)
     {
         auto &dl = dls[i];
         (*errnos)[i] = dl->dl_errno;
         if (dl->dl_errno != 0)
-        {
-            char buf[80];
-            // not sure why yet, but valgrind complains about the call below
             logger->log(LOG_ERR, "Downloader: failed to download %s, got '%s'", keys[i]->c_str(), strerror_r(dl->dl_errno, buf, 80));
-            ret = -1;
-        }
     }
 }
 
@@ -132,7 +127,11 @@ void Downloader::Download::operator()()
     CloudStorage *storage = CloudStorage::get();
     int err = storage->getObject(*key, dler->getDownloadPath() + "/" + *key, &size);
     if (err != 0)
+    {
+        boost::filesystem::remove(dler->getDownloadPath() + "/" + *key);
+        size = 0;
         dl_errno = errno;
+    }
     
     boost::unique_lock<boost::mutex> s(dler->getDownloadMutex());
     for (auto &listener : listeners)

src/Downloader.h

@@ -22,9 +22,9 @@ class Downloader
         Downloader();
         virtual ~Downloader();
         
-        // returns 0 on success.  If != 0, errnos will contains the errno associated with the failure
         // caller owns the memory for the strings.
-        int download(const std::vector<const std::string *> &keys, std::vector<int> *errnos, std::vector<size_t> *sizes);
+        // errors are reported through errnos
+        void download(const std::vector<const std::string *> &keys, std::vector<int> *errnos, std::vector<size_t> *sizes);
         void setDownloadPath(const std::string &path);
         const std::string & getDownloadPath() const;
         

src/IOCoordinator.cpp

@@ -263,7 +263,7 @@ int IOCoordinator::write(const char *filename, const uint8_t *data, off_t offset
         for (std::vector<metadataObject>::const_iterator i = objects.begin(); i != objects.end(); ++i)
         {
             // figure out how much data to write to this object
-            if (count == 0 && offset > i->offset)
+            if (count == 0 && (uint64_t) offset > i->offset)
             {
                 // first object in the list so start at offset and
                 // write to end of oject or all the data
@@ -335,11 +335,13 @@ int IOCoordinator::append(const char *filename, const uint8_t *data, size_t leng
     size_t count = 0;
     while (count < length) {
         err = ::write(fd, &data[count], length - count);
-        if (err <= 0)
+        if (err <= 0) 
+        {
             if (count > 0)   // return what was successfully written
                 return count;
             else
                 return err;
+        }
         count += err;
     }
     
@@ -809,10 +811,10 @@ boost::shared_array<uint8_t> IOCoordinator::mergeJournal(const char *object, con
     
     if (*len == 0)
         // read to the end of the file
-        *len = max(maxJournalOffset, objStat.st_size) - offset;
+        *len = max(maxJournalOffset, (size_t) objStat.st_size) - offset;
     else
         // make sure len is within the bounds of the data
-        *len = min(*len, (max(maxJournalOffset, objStat.st_size) - offset));
+        *len = min(*len, (max(maxJournalOffset, (size_t) objStat.st_size) - offset));
     ret.reset(new uint8_t[*len]);
     
     // read the object into memory
@@ -852,9 +854,9 @@ boost::shared_array<uint8_t> IOCoordinator::mergeJournal(const char *object, con
         // if this entry overlaps, read the overlapping section
         uint64_t lastJournalOffset = offlen[0] + offlen[1];
         uint64_t lastBufOffset = offset + *len;
-        if (offlen[0] <= lastBufOffset && lastJournalOffset >= offset)
+        if (offlen[0] <= lastBufOffset && lastJournalOffset >= (uint64_t) offset)
         {
-            uint64_t startReadingAt = max(offlen[0], offset);
+            uint64_t startReadingAt = max(offlen[0], (uint64_t) offset);
             uint64_t lengthOfRead = min(lastBufOffset, lastJournalOffset) - startReadingAt;
             
             if (startReadingAt != offlen[0])
@@ -958,7 +960,7 @@ void IOCoordinator::renameObject(const string &oldKey, const string &newKey)
 }
 
 
-bool IOCoordinator::readLock(const string &filename)
+void IOCoordinator::readLock(const string &filename)
 {
     boost::unique_lock<boost::mutex> s(lockMutex);
 
@@ -981,7 +983,7 @@ void IOCoordinator::readUnlock(const string &filename)
     }
 }
 
-bool IOCoordinator::writeLock(const string &filename)
+void IOCoordinator::writeLock(const string &filename)
 {
     boost::unique_lock<boost::mutex> s(lockMutex);
     

src/IOCoordinator.h

@@ -59,8 +59,8 @@ class IOCoordinator : public boost::noncopyable
         /* Lock manipulation fcns.  They can lock on any param given to them.  For convention's sake,
            the parameter should mostly be the abs filename being accessed. */
         void renameObject(const std::string &oldKey, const std::string &newKey);
-        bool readLock(const std::string &filename);
+        void readLock(const std::string &filename);
-        bool writeLock(const std::string &filename);
+        void writeLock(const std::string &filename);
         void readUnlock(const std::string &filename);
         void writeUnlock(const std::string &filename);
         

src/ListDirectoryTask.cpp

@@ -37,7 +37,7 @@ bool ListDirectoryTask::writeString(uint8_t *buf, int *offset, int size, const s
         check_error("ListDirectoryTask::writeString()", false);
         *offset = 0;
     }
-    uint count = 0, len = str.length();
+    int count = 0, len = str.length();
     *((uint32_t *) &buf[*offset]) = len;
     *offset += 4;
     while (count < len)

src/LocalStorage.cpp

@@ -72,7 +72,6 @@ int LocalStorage::getObject(const string &source, const string &dest, size_t *si
 
 int LocalStorage::getObject(const std::string &sourceKey, boost::shared_array<uint8_t> *data, size_t *size)
 {
-    int ret;
     bf::path source = prefix / sourceKey;
     const char *c_source = source.string().c_str();
     char buf[80];

src/MetadataFile.cpp

@@ -225,7 +225,7 @@ vector<metadataObject> MetadataFile::metadataRead(off_t offset, size_t length) c
     // find the first object in range
     while (i != mObjects.end())
     {
-        if (offset <= (i->offset + i->length - 1))
+        if ((uint64_t) offset <= (i->offset + i->length - 1))
             break;
         ++i;
     }
@@ -313,7 +313,7 @@ string MetadataFile::getNewKey(string sourceName, size_t offset, size_t length)
     boost::uuids::uuid u = boost::uuids::random_generator()();
     stringstream ss;
 
-    for (int i = 0; i < sourceName.length(); i++)
+    for (uint i = 0; i < sourceName.length(); i++)
     {
         if (sourceName[i] == '/')
         {
@@ -339,7 +339,7 @@ string MetadataFile::getSourceFromKey(const string &key)
     
     // this is to convert the munged filenames back to regular filenames
     // for consistent use in IOC locks
-    for (int i = 0; i < split[3].length(); i++)
+    for (uint i = 0; i < split[3].length(); i++)
         if (split[3][i] == '~')
             split[3][i] = '/';
     

src/ReadTask.cpp

@@ -58,7 +58,7 @@ bool ReadTask::run()
     // todo: need to make this use O_DIRECT on the IOC side
     ioc->willRead(cmd->filename, cmd->offset, cmd->count);
     int err;
-    while (resp->returnCode < cmd->count)
+    while ((uint) resp->returnCode < cmd->count)
     {
         try
         {

src/Replicator.cpp

@@ -94,10 +94,12 @@ int Replicator::newObject(const char *filename, const uint8_t *data, size_t leng
     while (count < length) {
         err = ::write(fd, &data[count], length - count);
         if (err <= 0)
+        {
             if (count > 0)   // return what was successfully written
                 return count;
             else
                 return err;
+        }
         count += err;
     }
 
@@ -107,7 +109,7 @@ int Replicator::newObject(const char *filename, const uint8_t *data, size_t leng
 int Replicator::addJournalEntry(const char *filename, const uint8_t *data, off_t offset, size_t length)
 {
     int fd, err;
-    uint64_t offlen[] = {offset,length};
+    uint64_t offlen[] = {(uint64_t) offset,length};
     size_t count = 0;
     int version = 1;
     string journalFilename = msJournalPath + "/" + string(filename) + ".journal";
@@ -150,10 +152,12 @@ int Replicator::addJournalEntry(const char *filename, const uint8_t *data, off_t
     while (count < length) {
         err = ::write(fd, &data[count], length - count);
         if (err <= 0)
+        {
             if (count > 0)   // return what was successfully written
                 return count;
             else
                 return err;
+        }
         count += err;
     }
 

src/SessionManager.cpp

@@ -231,9 +231,7 @@ int SessionManager::start()
                     uint remainingBytes = 0;
                     uint endOfData, i;
                     int peakLength,len;
-                    struct timespec ts;
+                    
-                    ts.tv_sec = 0;
-                    ts.tv_nsec = 100000000;   // .1 sec
                     //logger->log(LOG_DEBUG,"reading from fd %i index is %i", fds[socketIncr].fd,socketIncr);
                     if (sockState.find(fds[socketIncr].fd) != sockState.end())
                     {

src/Utilities.cpp

@@ -4,7 +4,7 @@
 namespace storagemanager
 {
 
-ScopedReadLock::ScopedReadLock(IOCoordinator *i, const std::string &k) : ioc(i), key(k), locked(false)
+ScopedReadLock::ScopedReadLock(IOCoordinator *i, const std::string &k) : ioc(i), locked(false), key(k)
 {
     lock();
 }
@@ -29,7 +29,7 @@ void ScopedReadLock::unlock()
     }
 }
 
-ScopedWriteLock::ScopedWriteLock(IOCoordinator *i, const std::string &k) : ioc(i), key(k), locked(false)
+ScopedWriteLock::ScopedWriteLock(IOCoordinator *i, const std::string &k) : ioc(i), locked(false), key(k)
 {
     lock();
 }

src/unit_tests.cpp

@@ -125,7 +125,7 @@ void acceptConnection()
         sa.sun_family = AF_UNIX;
         memcpy(&sa.sun_path[1], "testing", 7);
         
-        int err = ::bind(serverSock, (struct sockaddr *) &sa, sizeof(sa));
+        err = ::bind(serverSock, (struct sockaddr *) &sa, sizeof(sa));
         assert(err == 0);
         err = ::listen(serverSock, 2);
         assert(err == 0);
@@ -208,7 +208,6 @@ bool replicatorTest()
     Replicator *repli = Replicator::get();
     int err,fd;
     const char *newobject = "newobjectTest";
-    const char *newobjectJournal = "newobjectTest.journal";
     string newObjectJournalFullPath = journalPath + "/" + "newobjectTest.journal";
     uint8_t buf[1024];
     uint8_t data[1024];
@@ -234,7 +233,7 @@ bool replicatorTest()
 
     fd = ::open(newObjectJournalFullPath.c_str(), O_RDONLY);
     err = ::read(fd, buf, 1024);
-    assert(err == (header.length() + 1 + 16 + 10));
+    assert((uint) err == (header.length() + 1 + 16 + 10));
     buf[err] = 0;
     assert(!strcmp("1234567890", (const char *) buf + header.length() + 1 + 16));
     cout << "replicator addJournalEntry OK" << endl;
@@ -271,22 +270,22 @@ bool metadataJournalTest(std::size_t size, off_t offset)
     hdr->type = SM_MSG_START;
     hdr->payloadLen = sizeof(*cmd) + cmd->flen + cmd->count;
     WriteTask w(clientSock, hdr->payloadLen);
-    int error = ::write(sessionSock, cmd, hdr->payloadLen);
+    int err = ::write(sessionSock, cmd, hdr->payloadLen);
 
     w.run();
 
     // verify response
-    int err = ::recv(sessionSock, buf, 1024, MSG_DONTWAIT);
+    err = ::recv(sessionSock, buf, 1024, MSG_DONTWAIT);
     sm_response *resp = (sm_response *) buf;
     assert(err == sizeof(*resp));
     assert(resp->header.type == SM_MSG_START);
     assert(resp->header.payloadLen == 4);
     assert(resp->header.flags == 0);
-    assert(resp->returnCode == size);
+    assert(resp->returnCode == (int) size);
 
     MetadataFile mdfTest(filename);
     mdfTest.printObjects();
-
+    return true;
 }
 
 void metadataJournalTestCleanup(std::size_t size)
@@ -405,7 +404,7 @@ bool appendtask()
     return true;
 }
 
-bool unlinktask()
+void unlinktask()
 {
     // make a meta file and delete it
     const char *filename = "unlinktest1";
@@ -418,7 +417,6 @@ bool unlinktask()
     
     uint8_t buf[1024];
     unlink_cmd *cmd = (unlink_cmd *) buf;
-    uint8_t *data;
     
     cmd->opcode = UNLINK;
     cmd->flen = strlen(filename);
@@ -721,7 +719,7 @@ bool listdirtask()
     listdir_resp *r = (listdir_resp *) resp->payload;
     assert(r->elements == 10);
     int off = sizeof(sm_response) + sizeof(listdir_resp);
-    int fileCounter = 0;
+    uint fileCounter = 0;
     while (off < err)
     {
         listdir_resp_entry *e = (listdir_resp_entry *) &buf[off];
@@ -738,7 +736,7 @@ bool listdirtask()
     return true;
 }
 
-bool pingtask()
+void pingtask()
 {
     uint8_t buf[1024];
     ping_cmd *cmd = (ping_cmd *) buf;
@@ -850,7 +848,7 @@ bool cacheTest1()
     cache->exists(v_bogus, &exists);
     assert(exists.size() == 1);
     assert(exists[0]);
-    ssize_t currentSize = cache->getCurrentCacheSize();
+    size_t currentSize = cache->getCurrentCacheSize();
     assert(currentSize == bf::file_size(cachePath / realFile));
     
     // lie about the file being deleted and then replaced
@@ -866,10 +864,9 @@ bool cacheTest1()
     bf::remove(cachePath / realFile);
     bf::remove(storagePath / realFile);
     cout << "cache test 1 OK" << endl;
+    return true;
 }
 
-
-
 bool mergeJournalTest()
 {
     /*
@@ -1005,7 +1002,6 @@ bool syncTest1()
         foundIt = (MetadataFile::getSourceFromKey(newKey) == "test-file");
         if (foundIt)
         {
-            size_t fsize = bf::file_size(dir->path());
             assert(cache->exists(newKey));
             cs->deleteObject(newKey);
             break;
@@ -1120,9 +1116,9 @@ void s3storageTest1()
         uint8_t *data1 = new uint8_t[len];
         uint8_t *data2 = new uint8_t[len];
         err = read(fd1, data1, len);
-        assert(err == len);
+        assert(err == (int) len);
         err = read(fd2, data2, len);
-        assert(err == len);
+        assert(err == (int) len);
         assert(!memcmp(data1, data2, len));
         close(fd1);
         close(fd2);
@@ -1194,7 +1190,7 @@ void IOCReadTest1()
     makeTestMetadata(metaFilename.c_str());
     size_t objSize = bf::file_size(objFilename);
     err = ioc->read(testFile, data.get(), 0, 1<<20);
-    assert(err == objSize);
+    assert(err == (int) objSize);
     
     // verify the data
     int *data32 = (int *) data.get();
@@ -1207,7 +1203,7 @@ void IOCReadTest1()
     makeTestJournal(journalFilename.c_str());
     
     err = ioc->read(testFile, data.get(), 0, 1<<20);
-    assert(err == objSize);
+    assert(err == (int) objSize);
     for (i = 0; i < 5; i++)
         assert(data32[i] == i);
     for (; i < 10; i++)
@@ -1362,9 +1358,8 @@ void IOCCopyFile3()
         call ioc::copyFile()
         verify dest file exists
     */
-        IOCoordinator *ioc = IOCoordinator::get();
+    IOCoordinator *ioc = IOCoordinator::get();
     Cache *cache = Cache::get();
-    CloudStorage *cs = CloudStorage::get();
     
     bf::path metaPath = ioc->getMetadataPath();
     bf::path journalPath = ioc->getJournalPath();