Merge "Fix native crash by caused wrong munmap."

main
Keisuke Kuroyanagi 2013-08-21 10:11:48 +00:00 committed by Android (Google) Code Review
commit 2617a82a98
4 changed files with 37 additions and 34 deletions

View File

@ -22,7 +22,7 @@
#include "suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h" #include "suggest/policyimpl/dictionary/dynamic_patricia_trie_policy.h"
#include "suggest/policyimpl/dictionary/patricia_trie_policy.h" #include "suggest/policyimpl/dictionary/patricia_trie_policy.h"
#include "suggest/policyimpl/dictionary/utils/format_utils.h" #include "suggest/policyimpl/dictionary/utils/format_utils.h"
#include "suggest/policyimpl/dictionary/utils/mmaped_buffer.h" #include "suggest/policyimpl/dictionary/utils/mmapped_buffer.h"
namespace latinime { namespace latinime {
@ -31,7 +31,7 @@ namespace latinime {
const int bufOffset, const int size, const bool isUpdatable) { const int bufOffset, const int size, const bool isUpdatable) {
// Allocated buffer in MmapedBuffer::openBuffer() will be freed in the destructor of // Allocated buffer in MmapedBuffer::openBuffer() will be freed in the destructor of
// impl classes of DictionaryStructureWithBufferPolicy. // impl classes of DictionaryStructureWithBufferPolicy.
const MmapedBuffer *const mmapedBuffer = MmapedBuffer::openBuffer(path, pathLength, bufOffset, const MmappedBuffer *const mmapedBuffer = MmappedBuffer::openBuffer(path, pathLength, bufOffset,
size, isUpdatable); size, isUpdatable);
if (!mmapedBuffer) { if (!mmapedBuffer) {
return 0; return 0;

View File

@ -24,7 +24,7 @@
#include "suggest/policyimpl/dictionary/bigram/bigram_list_policy.h" #include "suggest/policyimpl/dictionary/bigram/bigram_list_policy.h"
#include "suggest/policyimpl/dictionary/header/header_policy.h" #include "suggest/policyimpl/dictionary/header/header_policy.h"
#include "suggest/policyimpl/dictionary/shortcut/shortcut_list_policy.h" #include "suggest/policyimpl/dictionary/shortcut/shortcut_list_policy.h"
#include "suggest/policyimpl/dictionary/utils/mmaped_buffer.h" #include "suggest/policyimpl/dictionary/utils/mmapped_buffer.h"
namespace latinime { namespace latinime {
@ -33,7 +33,7 @@ class DicNodeVector;
class DynamicPatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { class DynamicPatriciaTriePolicy : public DictionaryStructureWithBufferPolicy {
public: public:
DynamicPatriciaTriePolicy(const MmapedBuffer *const buffer) DynamicPatriciaTriePolicy(const MmappedBuffer *const buffer)
: mBuffer(buffer), mHeaderPolicy(mBuffer->getBuffer()), : mBuffer(buffer), mHeaderPolicy(mBuffer->getBuffer()),
mDictRoot(mBuffer->getBuffer() + mHeaderPolicy.getSize()), mDictRoot(mBuffer->getBuffer() + mHeaderPolicy.getSize()),
mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot) {} mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot) {}
@ -86,7 +86,7 @@ class DynamicPatriciaTriePolicy : public DictionaryStructureWithBufferPolicy {
DISALLOW_IMPLICIT_CONSTRUCTORS(DynamicPatriciaTriePolicy); DISALLOW_IMPLICIT_CONSTRUCTORS(DynamicPatriciaTriePolicy);
static const int MAX_CHILD_COUNT_TO_AVOID_INFINITE_LOOP; static const int MAX_CHILD_COUNT_TO_AVOID_INFINITE_LOOP;
const MmapedBuffer *const mBuffer; const MmappedBuffer *const mBuffer;
const HeaderPolicy mHeaderPolicy; const HeaderPolicy mHeaderPolicy;
// TODO: Consolidate mDictRoot. // TODO: Consolidate mDictRoot.
const uint8_t *const mDictRoot; const uint8_t *const mDictRoot;

View File

@ -24,7 +24,7 @@
#include "suggest/policyimpl/dictionary/bigram/bigram_list_policy.h" #include "suggest/policyimpl/dictionary/bigram/bigram_list_policy.h"
#include "suggest/policyimpl/dictionary/header/header_policy.h" #include "suggest/policyimpl/dictionary/header/header_policy.h"
#include "suggest/policyimpl/dictionary/shortcut/shortcut_list_policy.h" #include "suggest/policyimpl/dictionary/shortcut/shortcut_list_policy.h"
#include "suggest/policyimpl/dictionary/utils/mmaped_buffer.h" #include "suggest/policyimpl/dictionary/utils/mmapped_buffer.h"
namespace latinime { namespace latinime {
@ -33,7 +33,7 @@ class DicNodeVector;
class PatriciaTriePolicy : public DictionaryStructureWithBufferPolicy { class PatriciaTriePolicy : public DictionaryStructureWithBufferPolicy {
public: public:
PatriciaTriePolicy(const MmapedBuffer *const buffer) PatriciaTriePolicy(const MmappedBuffer *const buffer)
: mBuffer(buffer), mHeaderPolicy(mBuffer->getBuffer()), : mBuffer(buffer), mHeaderPolicy(mBuffer->getBuffer()),
mDictRoot(mBuffer->getBuffer() + mHeaderPolicy.getSize()), mDictRoot(mBuffer->getBuffer() + mHeaderPolicy.getSize()),
mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot) {} mBigramListPolicy(mDictRoot), mShortcutListPolicy(mDictRoot) {}
@ -97,7 +97,7 @@ class PatriciaTriePolicy : public DictionaryStructureWithBufferPolicy {
private: private:
DISALLOW_IMPLICIT_CONSTRUCTORS(PatriciaTriePolicy); DISALLOW_IMPLICIT_CONSTRUCTORS(PatriciaTriePolicy);
const MmapedBuffer *const mBuffer; const MmappedBuffer *const mBuffer;
const HeaderPolicy mHeaderPolicy; const HeaderPolicy mHeaderPolicy;
const uint8_t *const mDictRoot; const uint8_t *const mDictRoot;
const BigramListPolicy mBigramListPolicy; const BigramListPolicy mBigramListPolicy;

View File

@ -14,8 +14,8 @@
* limitations under the License. * limitations under the License.
*/ */
#ifndef LATINIME_MMAPED_BUFFER_H #ifndef LATINIME_MMAPPED_BUFFER_H
#define LATINIME_MMAPED_BUFFER_H #define LATINIME_MMAPPED_BUFFER_H
#include <cerrno> #include <cerrno>
#include <fcntl.h> #include <fcntl.h>
@ -27,39 +27,40 @@
namespace latinime { namespace latinime {
class MmapedBuffer { class MmappedBuffer {
public: public:
static MmapedBuffer* openBuffer(const char *const path, const int pathLength, static MmappedBuffer* openBuffer(const char *const path, const int pathLength,
const int bufOffset, const int size, const bool isUpdatable) { const int bufferOffset, const int bufferSize, const bool isUpdatable) {
const int openMode = isUpdatable ? O_RDWR : O_RDONLY; const int openMode = isUpdatable ? O_RDWR : O_RDONLY;
const int fd = open(path, openMode); const int mmapFd = open(path, openMode);
if (fd < 0) { if (mmapFd < 0) {
AKLOGE("DICT: Can't open the source. path=%s errno=%d", path, errno); AKLOGE("DICT: Can't open the source. path=%s errno=%d", path, errno);
return 0; return 0;
} }
const int pagesize = getpagesize(); const int pagesize = getpagesize();
const int offset = bufOffset % pagesize; const int offset = bufferOffset % pagesize;
int adjOffset = bufOffset - offset; int alignedOffset = bufferOffset - offset;
int adjSize = size + offset; int alignedSize = bufferSize + offset;
const int protMode = isUpdatable ? PROT_READ | PROT_WRITE : PROT_READ; const int protMode = isUpdatable ? PROT_READ | PROT_WRITE : PROT_READ;
void *const mmapedBuffer = mmap(0, adjSize, protMode, MAP_PRIVATE, fd, adjOffset); void *const mmappedBuffer = mmap(0, alignedSize, protMode, MAP_PRIVATE, mmapFd,
if (mmapedBuffer == MAP_FAILED) { alignedOffset);
if (mmappedBuffer == MAP_FAILED) {
AKLOGE("DICT: Can't mmap dictionary. errno=%d", errno); AKLOGE("DICT: Can't mmap dictionary. errno=%d", errno);
close(fd); close(mmapFd);
return 0; return 0;
} }
uint8_t *const buffer = static_cast<uint8_t *>(mmapedBuffer) + offset; uint8_t *const buffer = static_cast<uint8_t *>(mmappedBuffer) + offset;
if (!buffer) { if (!buffer) {
AKLOGE("DICT: buffer is null"); AKLOGE("DICT: buffer is null");
close(fd); close(mmapFd);
return 0; return 0;
} }
return new MmapedBuffer(buffer, adjSize, fd, offset, isUpdatable); return new MmappedBuffer(buffer, bufferSize, mmappedBuffer, alignedSize, mmapFd,
isUpdatable);
} }
~MmapedBuffer() { ~MmappedBuffer() {
int ret = munmap(static_cast<void *>(mBuffer - mBufferOffset), int ret = munmap(mMmappedBuffer, mAlignedSize);
mBufferSize + mBufferOffset);
if (ret != 0) { if (ret != 0) {
AKLOGE("DICT: Failure in munmap. ret=%d errno=%d", ret, errno); AKLOGE("DICT: Failure in munmap. ret=%d errno=%d", ret, errno);
} }
@ -82,18 +83,20 @@ class MmapedBuffer {
} }
private: private:
AK_FORCE_INLINE MmapedBuffer(uint8_t *const buffer, const int bufferSize, const int mmapFd, AK_FORCE_INLINE MmappedBuffer(uint8_t *const buffer, const int bufferSize,
const int bufferOffset, const bool isUpdatable) void *const mmappedBuffer, const int alignedSize, const int mmapFd,
: mBuffer(buffer), mBufferSize(bufferSize), mMmapFd(mmapFd), const bool isUpdatable)
mBufferOffset(bufferOffset), mIsUpdatable(isUpdatable) {} : mBuffer(buffer), mBufferSize(bufferSize), mMmappedBuffer(mmappedBuffer),
mAlignedSize(alignedSize), mMmapFd(mmapFd), mIsUpdatable(isUpdatable) {}
DISALLOW_IMPLICIT_CONSTRUCTORS(MmapedBuffer); DISALLOW_IMPLICIT_CONSTRUCTORS(MmappedBuffer);
uint8_t *const mBuffer; uint8_t *const mBuffer;
const int mBufferSize; const int mBufferSize;
void *const mMmappedBuffer;
const int mAlignedSize;
const int mMmapFd; const int mMmapFd;
const int mBufferOffset;
const bool mIsUpdatable; const bool mIsUpdatable;
}; };
} }
#endif /* LATINIME_MMAPED_BUFFER_H */ #endif /* LATINIME_MMAPPED_BUFFER_H */