Issue 6241 - Allow a native build.

compiled/library.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus 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 Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 #include <cwctype>
+#include <mutex>
 #include <regex>
 
 #include "String.h"
+#include "Utils.h"
+#include "debug.h"
 #include "library.h"
 
 
 char16_t CharToLower(char16_t charCode)
 {
   return std::towlower(charCode);
 }
 
 
 void JSNotifyFilterChange(FilterNotifier::Topic topic, Filter& filter,
       Subscription* subscription, unsigned int position)
 {

[sergei, 2018/01/11 08:54:07]

stubs are OK for present.

 }
 
 void JSNotifySubscriptionChange(FilterNotifier::Topic topic,
       Subscription& subscription)
 {
 }
 
 namespace {
-  std::vector<std::unique_ptr<std::wregex>> mRegexPool;

[sergei, 2018/01/11 08:54:06]

BTW, this code is not thread safe, we should be very careful with that because
libadblockplus is multithreaded.
I'm not sure whether we should already add some serializing the access mechanism
because there is plenty of other places like this, so maybe it should be
addressed at another level or uniformly but later.

[hub, 2018/01/11 20:41:27]

Done.

-
-  std::wstring from_string(const String& str)

[sergei, 2018/01/11 08:54:07]

What about putting this method into some utils.h (possible cpp) file which is
used in the native builds because it seems we will need it more than only here?
BTW, IMO it should be named, something like StdWStringFromString.

[hub, 2018/01/11 20:41:27]

Done.

-  {
-    std::wstring s;
-    for (String::size_type idx = 0; idx < str.length(); idx++)
-      s.push_back(str[idx]);

[sergei, 2018/01/11 08:54:07]

std::string has the constructor accepting a pointer to an array of characters
and its size, IMO it would be better to use it here, e.g.
return std::wstring(str.data(), str.length());

[hub, 2018/01/11 20:41:27]

std::wstring::value_type is a wchar_t which is 32-bits (Windows might be the
exception), so this can't work.

Also we can't use std::u16string as char16_t doesn't have a std::ctype
implementation provided.

However std::wstring s(str.data(), str.data() + str.length()) will work
(iterator range).

[sergei, 2018/01/12 09:58:12]

Acknowledged.

-
-    return s;
-  }
+  std::vector<std::unique_ptr<std::wregex>> regexPool;

[sergei, 2018/01/12 09:58:12]

I missed it first time, but what is the whole point of it? Why do we need such
pool? GenerateRegExp should just return std::wregex, DeleteRegExp should do
nothing and TestRegExp should use std::wregex.

[hub, 2018/01/12 14:33:53]

This is because of the original code and bindings in the emscripten branch
(library.js) ; this is meant to be a drop in replacement.

(I'm not a fan of it either)

+  std::mutex regexPoolMutex;
 }
 
 int GenerateRegExp(const String& regexp, bool matchCase)
 {
-  auto index = mRegexPool.size();
+  std::lock_guard<std::mutex> guard(regexPoolMutex);
+  auto index = regexPool.size();
   auto flags = std::regex_constants::ECMAScript;
   if (!matchCase)
     flags |= std::regex_constants::icase;
-  mRegexPool.emplace_back(new std::wregex(from_string(regexp), flags));
+  regexPool.emplace_back(new std::wregex(StdWStringFromString(regexp), flags));
   return index;
 }
 
 void DeleteRegExp(int id)
 {
-  if (id < mRegexPool.size())
-    mRegexPool[id].reset();

[sergei, 2018/01/11 08:54:07]

What about using of std::list and its iterator as ID, one can remove an element
without invalidation of other iterators. BTW in this case there will not be the
necessity in a smart pointer.

[hub, 2018/01/11 20:41:27]

How do you know if the id is valid? erase() invalidate the iterator of the
erased item, and there is no way to check if it is derefenceable.

[sergei, 2018/01/12 09:58:12]

IMO there is no necessity in it, as soon as the regexp is removed, its ID is not
supposed to be used, so it's fine if we just throw an exception or the app
crashes :).

+  std::lock_guard<std::mutex> guard(regexPoolMutex);
+  if (id < regexPool.size())
+    regexPool[id].reset();
 }
 
 bool TestRegExp(int id, const String& str)
 {
-  if ((id < mRegexPool.size()) && mRegexPool[id])
-    return std::regex_match(from_string(str), *mRegexPool[id]);
+  std::lock_guard<std::mutex> guard(regexPoolMutex);
+  if ((id < regexPool.size()) && regexPool[id])
+    return std::regex_match(StdWStringFromString(str), *regexPool[id]);
 
+  assert2(id < regexPool.size() && regexPool[id], "Invalid RegExp index.");
   return false;

[sergei, 2018/01/11 08:54:07]

Could please add an assert here because it should never happen that id is
invalid.

[hub, 2018/01/11 20:41:27]

Done.

 }