Kelson has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/295518 )

Change subject: Use a Queue object to handle threadsafe access to a queue.
......................................................................


Use a Queue object to handle threadsafe access to a queue.

By using a Queue object we avoid the declaration of popFromFilenameQueue
function in articlesource.cpp.

Change-Id: Ic685f7e22e4ce95f6e0eb280f65809fd0dff1a6a
---
M zimwriterfs/articlesource.cpp
M zimwriterfs/articlesource.h
A zimwriterfs/queue.h
M zimwriterfs/zimwriterfs.cpp
4 files changed, 116 insertions(+), 55 deletions(-)

Approvals:
  Kelson: Verified; Looks good to me, approved



diff --git a/zimwriterfs/articlesource.cpp b/zimwriterfs/articlesource.cpp
index 8b0b34c..6cf5f30 100644
--- a/zimwriterfs/articlesource.cpp
+++ b/zimwriterfs/articlesource.cpp
@@ -28,7 +28,6 @@
 #include <sstream>
 #include <map>
 
-bool popFromFilenameQueue(std::string &filename);
 bool isVerbose();
 
 extern std::string welcome;
@@ -44,8 +43,9 @@
 unsigned int dataSize = 0;
 
 
-
-ArticleSource::ArticleSource() {
+ArticleSource::ArticleSource(Queue<std::string>& filenameQueue):
+    filenameQueue(filenameQueue)
+{
   /* Prepare metadata */
   metadataQueue.push("Language");
   metadataQueue.push("Publisher");
@@ -88,10 +88,10 @@
     std::string line = redirectsQueue.front();
     redirectsQueue.pop();
     article = new RedirectArticle(line);
-  } else if (popFromFilenameQueue(path)) {
+  } else if (filenameQueue.popFromQueue(path)) {
     do {
       article = new Article(path);
-    } while (article && article->isInvalid() && popFromFilenameQueue(path));
+    } while (article && article->isInvalid() && 
filenameQueue.popFromQueue(path));
   } else {
     article = NULL;
   }
diff --git a/zimwriterfs/articlesource.h b/zimwriterfs/articlesource.h
index adbdbda..1ad6524 100644
--- a/zimwriterfs/articlesource.h
+++ b/zimwriterfs/articlesource.h
@@ -24,12 +24,13 @@
 #include <string>
 #include <queue>
 #include <fstream>
+#include "queue.h"
 
 #include <zim/writer/zimcreator.h>
 
 class ArticleSource : public zim::writer::ArticleSource {
   public:
-    explicit ArticleSource();
+    explicit ArticleSource(Queue<std::string>& filenameQueue);
     virtual const zim::writer::Article* getNextArticle();
     virtual zim::Blob getData(const std::string& aid);
     virtual std::string getMainPage();
@@ -39,6 +40,7 @@
   private:
     std::queue<std::string> metadataQueue;
     std::queue<std::string> redirectsQueue;
+    Queue<std::string>&     filenameQueue;
 };
 
 #endif //OPENZIM_ZIMWRITERFS_ARTICLESOURCE_H
diff --git a/zimwriterfs/queue.h b/zimwriterfs/queue.h
new file mode 100644
index 0000000..d177568
--- /dev/null
+++ b/zimwriterfs/queue.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2016 Matthieu Gautier <mgaut...@kymeria.fr>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU  General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ */
+
+#ifndef OPENZIM_ZIMWRITERFS_QUEUE_H
+#define OPENZIM_ZIMWRITERFS_QUEUE_H
+
+#define MAX_QUEUE_SIZE 100
+
+#include <pthread.h>
+#include <unistd.h>
+
+template<typename T>
+class Queue {
+    public:
+        Queue() {pthread_mutex_init(&m_queueMutex,NULL);};
+        virtual ~Queue() {pthread_mutex_destroy(&m_queueMutex);};
+        virtual bool isEmpty();
+        virtual void pushToQueue(const T& element);
+        virtual bool popFromQueue(T &filename);
+
+    protected:
+        std::queue<T>   m_realQueue;
+        pthread_mutex_t m_queueMutex;
+
+    private:
+        // Make this queue non copyable
+        Queue(const Queue&);
+        Queue& operator=(const Queue&);
+};
+
+template<typename T>
+bool Queue<T>::isEmpty() {
+    pthread_mutex_lock(&m_queueMutex);
+    bool retVal = m_realQueue.empty();
+    pthread_mutex_unlock(&m_queueMutex);
+    return retVal;
+}
+
+template<typename T>
+void Queue<T>::pushToQueue(const T &element) {
+    unsigned int wait = 0;
+    unsigned int queueSize = 0;
+
+    do {
+        usleep(wait);
+        pthread_mutex_lock(&m_queueMutex);
+        queueSize = m_realQueue.size();
+        pthread_mutex_unlock(&m_queueMutex);
+        wait += 10;
+    } while (queueSize > MAX_QUEUE_SIZE);
+
+    pthread_mutex_lock(&m_queueMutex);
+    m_realQueue.push(element);
+    pthread_mutex_unlock(&m_queueMutex);
+}
+
+template<typename T>
+bool Queue<T>::popFromQueue(T &element) {
+    pthread_mutex_lock(&m_queueMutex);
+    if (m_realQueue.empty()) {
+        pthread_mutex_unlock(&m_queueMutex);
+        return false;
+    }
+
+    element = m_realQueue.front();
+    m_realQueue.pop();
+    pthread_mutex_unlock(&m_queueMutex);
+
+  return true;
+}
+
+#endif // OPENZIM_ZIMWRITERFS_QUEUE_H
\ No newline at end of file
diff --git a/zimwriterfs/zimwriterfs.cpp b/zimwriterfs/zimwriterfs.cpp
index de44cb8..09a62af 100644
--- a/zimwriterfs/zimwriterfs.cpp
+++ b/zimwriterfs/zimwriterfs.cpp
@@ -35,8 +35,7 @@
 #include "tools.h"
 #include "article.h"
 #include "articlesource.h"
-
-#define MAX_QUEUE_SIZE 100
+#include "queue.h"
 
 std::string language;
 std::string creator;
@@ -50,8 +49,6 @@
 std::string zimPath;
 zim::writer::ZimCreator zimCreator;
 pthread_t directoryVisitor;
-pthread_mutex_t filenameQueueMutex;
-std::queue<std::string> filenameQueue;
 
 bool isDirectoryVisitorRunningFlag = false;
 pthread_mutex_t directoryVisitorRunningMutex;
@@ -83,50 +80,26 @@
   return retVal;
 }
 
-bool isFilenameQueueEmpty() {
-  pthread_mutex_lock(&filenameQueueMutex);
-  bool retVal = filenameQueue.empty();
-  pthread_mutex_unlock(&filenameQueueMutex);
-  return retVal;
-}
 
-void pushToFilenameQueue(const std::string &filename) {
-  unsigned int wait = 0;
-  unsigned int queueSize = 0;
+class FilenameQueue: public Queue<std::string> {
+    bool popFromQueue(std::string &filename) {
+        bool retVal = false;
+        unsigned int wait = 0;
 
-  do {
-    usleep(wait);
-    pthread_mutex_lock(&filenameQueueMutex);
-    unsigned queueSize = filenameQueue.size();
-    pthread_mutex_unlock(&filenameQueueMutex);
-    wait += 10;
-  } while (queueSize > MAX_QUEUE_SIZE);
+        do {
+            usleep(wait);
+            retVal = Queue::popFromQueue(filename);
+            if (retVal) {
+                break;
+            }
+            wait += 10;
+        } while (isDirectoryVisitorRunning() || !isEmpty());
 
-  pthread_mutex_lock(&filenameQueueMutex);
-  filenameQueue.push(filename);
-  pthread_mutex_unlock(&filenameQueueMutex); 
-}
-
-bool popFromFilenameQueue(std::string &filename) {
-  bool retVal = false;
-  unsigned int wait = 0;
-
-  do {
-    usleep(wait);
-    if (!isFilenameQueueEmpty()) {
-      pthread_mutex_lock(&filenameQueueMutex);
-      filename = filenameQueue.front();
-      filenameQueue.pop();
-      pthread_mutex_unlock(&filenameQueueMutex);
-      retVal = true;
-      break;
-    } else {
-      wait += 10;
+        return retVal;
     }
-  } while (isDirectoryVisitorRunning() || !isFilenameQueueEmpty());
+};
 
-  return retVal;
-}
+FilenameQueue filenameQueue;
 
 /* Non ZIM related code */
 void usage() {
@@ -196,7 +169,7 @@
 
       switch (entry->d_type) {
       case DT_REG:
-       pushToFilenameQueue(fullEntryName);
+       filenameQueue.pushToQueue(fullEntryName);
        break;
       case DT_DIR:
        visitDirectory(fullEntryName);
@@ -211,7 +184,7 @@
        std::cerr << "Unable to deal with " << fullEntryName << " (this is a 
named pipe)" << std::endl;
        break;
       case DT_LNK:
-       pushToFilenameQueue(fullEntryName);
+       filenameQueue.pushToQueue(fullEntryName);
        break;
       case DT_SOCK:
        std::cerr << "Unable to deal with " << fullEntryName << " (this is a 
UNIX domain socket)" << std::endl;
@@ -220,7 +193,7 @@
        struct stat s;
        if (stat(fullEntryName.c_str(), &s) == 0) {
          if (S_ISREG(s.st_mode)) {
-           pushToFilenameQueue(fullEntryName);
+           filenameQueue.pushToQueue(fullEntryName);
          } else if (S_ISDIR(s.st_mode)) {
            visitDirectory(fullEntryName);
           } else {
@@ -251,7 +224,7 @@
 }
 
 int main(int argc, char** argv) {
-  ArticleSource source;
+  ArticleSource source(filenameQueue);
   int minChunkSize = 2048;
   
 
@@ -371,7 +344,6 @@
   /* Init */
   magic = magic_open(MAGIC_MIME);
   magic_load(magic, NULL);
-  pthread_mutex_init(&filenameQueueMutex, NULL);
   pthread_mutex_init(&directoryVisitorRunningMutex, NULL);
   pthread_mutex_init(&verboseMutex, NULL);
 
@@ -392,5 +364,4 @@
   /* Destroy mutex */
   pthread_mutex_destroy(&directoryVisitorRunningMutex);
   pthread_mutex_destroy(&verboseMutex);
-  pthread_mutex_destroy(&filenameQueueMutex);
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/295518
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic685f7e22e4ce95f6e0eb280f65809fd0dff1a6a
Gerrit-PatchSet: 1
Gerrit-Project: openzim
Gerrit-Branch: master
Gerrit-Owner: Mgautierfr <mgaut...@kymeria.fr>
Gerrit-Reviewer: Kelson <kel...@kiwix.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to