Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ported regex files for Windows Phone 8.1 #22

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/boost/regex/v4/fileiter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,15 @@ class BOOST_REGEX_DECL mapfile_iterator
file = f;
node = f->_first + arg_position / mapfile::buf_size;
offset = arg_position % mapfile::buf_size;
file->lock(node);
if(file && node)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the assert earlier, these checks look redundant to me?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three functions:

  1. mapfile_iterator(const mapfile* f, long arg_position)
  2. mapfile_iterator(const mapfile_iterator& i)
  3. ~mapfile_iterator()

are not consistent in their manner of validity checking. If BOOST_ASSERT is like its C counterpart, then it will only catch a null 'f' value in debug, not release. For release mode, all validity checks must be in the form of C++ comparison statements. For that matter, the BOOST_ASSERT() macro should be used in all three functions.

In addition, the check of "if (file && node)" was being done in only function no. 1, not in 2 and 3. IMHO, the check should be done in all three functions (and not in 1 & 2 only as I submitted).

Both "file" and "node' should be checked for null values in all three functions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comment #19 for a similar discussion.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 03/11/2015 20:35, mosherubin wrote:

In include/boost/regex/v4/fileiter.hpp
#22 (comment):

@@ -229,14 +229,15 @@ class BOOST_REGEX_DECL mapfile_iterator
file = f;
node = f->_first + arg_position / mapfile::buf_size;
offset = arg_position % mapfile::buf_size;

  •  file->lock(node);
    
  •  if(file && node)
    

The three functions:

  1. mapfile_iterator(const mapfile* f, long arg_position)
  2. mapfile_iterator(const mapfile_iterator& i)
  3. ~mapfile_iterator()

are not consistent in their manner of validity checking. If
BOOST_ASSERT is like its C counterpart, then it will only catch a null
'f' value in debug, not release. For release mode, all validity checks
must be in the form of C++ comparison statements. For that matter, the
BOOST_ASSERT() macro should be used in all three functions.

In addition, the check of "if (file && node)" was being done in only
function no. 1, not in 2 and 3. IMHO, the check should be done in all
three functions (and not in 1 & 2 only as I submitted).

Both "file" and "node' should be checked for null values in all three
functions.

You need to realise that these are strictly internal implementation
details that are only constructed in certain ways (they're also
deprecated so we're arguing over angels on a pinhead, but never mind).

The following invariants hold:

if file is NULL then node is NULL.
If file is not NULL then node is not NULL.
When constructing from a mapfile then the argument is never NULL.

IMO using asserts to check these invariants is exactly the correct thing
to do.

Best, John.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi John,

I most certainly defer to you on these issues 👍 ! Do you need me to make any changes or can you make them? if the former, what would you like me to do?

Regards,

Moshe

file->lock(node);
}
mapfile_iterator(const mapfile_iterator& i)
{
file = i.file;
node = i.node;
offset = i.offset;
if(file)
if(file && node)
file->lock(node);
}
~mapfile_iterator()
Expand Down
44 changes: 41 additions & 3 deletions src/fileiter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ namespace std{
#include <sys/cygwin.h>
#endif

#if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use internal asio macros here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Indeed, the line should read:

#if defined (BOOST_PLAT_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)

(Note the use of 'PLAT' rather than 'ASIO'.) This must have leaked through from my port of ASIO and submitted to Chris Kohlhoff. :-)

I'd like to make the change to the #ifdef statement. I'm relatively new to Git: how can I make that change? Do I need to submit a new pull request?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to make the change to the #ifdef statement. I'm relatively
new to Git: how can I make that change? Do I need to submit a new pull
request?

No just push the change to the cloned repro you used for the PR and the
PR will get updated automatically.

Thanks, John.

WINBASEAPI HANDLE WINAPI CreateFileMappingFromAppW(HANDLE, LPSECURITY_ATTRIBUTES, ULONG, ULONG64, LPCWSTR);
WINBASEAPI LPVOID WINAPI MapViewOfFileFromAppW(HANDLE, ULONG, ULONG64, SIZE_T);
WINBASEAPI BOOL WINAPI UnmapViewOfFile(LPCVOID);
#endif /* defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE) */

#ifdef BOOST_MSVC
# pragma warning(disable: 4800)
#endif
Expand Down Expand Up @@ -87,13 +93,23 @@ const char* _fi_sep_alt = _fi_sep;

void mapfile::open(const char* file)
{
#if defined(BOOST_NO_ANSI_APIS)
#if defined(BOOST_NO_ANSI_APIS) || defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
int filename_size = strlen(file);
LPWSTR wide_file = (LPWSTR)_alloca( (filename_size + 1) * sizeof(WCHAR) );
if(::MultiByteToWideChar(CP_ACP, 0, file, filename_size, wide_file, filename_size + 1) == 0)
hfile = INVALID_HANDLE_VALUE;
else
{
# if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
CREATEFILE2_EXTENDED_PARAMETERS params = { 0 };

params.dwSize = sizeof (CREATEFILE2_EXTENDED_PARAMETERS);
params.dwFileAttributes = FILE_ATTRIBUTE_NORMAL;
hfile = CreateFile2(wide_file, GENERIC_READ, FILE_SHARE_READ, OPEN_EXISTING, &params);
# else
hfile = CreateFileW(wide_file, GENERIC_READ, FILE_SHARE_READ, 0, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0);
# endif
}
#elif defined(__CYGWIN__)||defined(__CYGWIN32__)
char win32file[ MAX_PATH ];
cygwin_conv_to_win32_path( file, win32file );
Expand All @@ -103,7 +119,11 @@ void mapfile::open(const char* file)
#endif
if(hfile != INVALID_HANDLE_VALUE)
{
#if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
hmap = CreateFileMappingFromAppW(hfile, 0, PAGE_READONLY, 0, 0);
#else
hmap = CreateFileMapping(hfile, 0, PAGE_READONLY, 0, 0, 0);
#endif
if((hmap == INVALID_HANDLE_VALUE) || (hmap == NULL))
{
CloseHandle(hfile);
Expand All @@ -112,7 +132,13 @@ void mapfile::open(const char* file)
std::runtime_error err("Unable to create file mapping.");
boost::BOOST_REGEX_DETAIL_NS::raise_runtime_error(err);
}

#if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
_first = static_cast<const char*>(MapViewOfFileFromAppW(hmap, FILE_MAP_READ, 0, 0));
#else
_first = static_cast<const char*>(MapViewOfFile(hmap, FILE_MAP_READ, 0, 0, 0));
#endif

if(_first == 0)
{
CloseHandle(hmap);
Expand All @@ -121,7 +147,14 @@ void mapfile::open(const char* file)
hfile = 0;
std::runtime_error err("Unable to create file mapping.");
}

#if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
WIN32_FILE_ATTRIBUTE_DATA fad = { 0 };
GetFileAttributesExW(wide_file, GetFileExInfoStandard, &fad);
_last = _first + fad.nFileSizeLow;
#else
_last = _first + GetFileSize(hfile, 0);
#endif
}
else
{
Expand Down Expand Up @@ -372,21 +405,26 @@ void mapfile::close()

inline _fi_find_handle find_first_file(const char* wild, _fi_find_data& data)
{
#ifdef BOOST_NO_ANSI_APIS
#if defined (BOOST_NO_ANSI_APIS) || defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
std::size_t wild_size = std::strlen(wild);
LPWSTR wide_wild = (LPWSTR)_alloca( (wild_size + 1) * sizeof(WCHAR) );
if (::MultiByteToWideChar(CP_ACP, 0, wild, wild_size, wide_wild, wild_size + 1) == 0)
return _fi_invalid_handle;

# if defined (BOOST_NO_ANSI_APIS)
return FindFirstFileW(wide_wild, &data);
# elif defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE)
return FindFirstFileExW(wide_wild, FindExInfoStandard, &data, FindExSearchNameMatch, NULL, 0);
# endif

#else
return FindFirstFileA(wild, &data);
#endif
}

inline bool find_next_file(_fi_find_handle hf, _fi_find_data& data)
{
#ifdef BOOST_NO_ANSI_APIS
#if defined (BOOST_NO_ANSI_APIS)
return FindNextFileW(hf, &data);
#else
return FindNextFileA(hf, &data);
Expand Down
8 changes: 8 additions & 0 deletions src/static_mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
#define BOOST_REGEX_SOURCE
#include <boost/config.hpp>
#include <boost/assert.hpp>
#if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE) || (WINAPI_FAMILY==WINAPI_FAMILY_PHONE_APP)
#include <chrono>
#include <thread>
#endif

#ifdef BOOST_HAS_THREADS

Expand Down Expand Up @@ -99,7 +103,11 @@ void scoped_static_mutex_lock::lock()
while(0 != InterlockedCompareExchange(reinterpret_cast<LONG*>(&(m_mutex.m_mutex)), 1, 0))
#endif
{
#if defined (BOOST_ASIO_WINDOWS_RUNTIME) || defined (BOOST_PLAT_WINDOWS_PHONE) || (WINAPI_FAMILY==WINAPI_FAMILY_PHONE_APP)
std::this_thread::sleep_for(std::chrono::milliseconds(1000));
#else
Sleep(0);
#endif
}
m_have_lock = true;
}
Expand Down
Loading