Skip to content

Commit

Permalink
Fixes memory allocation and cleanup bugs in QMatrix
Browse files Browse the repository at this point in the history
Summary:
1. When a QMatrix is created with the default constructor, codes_ is not allocated. Deconstructor should not attempt to delete codes in this case.
2. When a QMatrix is created with the parameterized constructor, computation of codesize_ is incorrect because std::ceil is used incorrectly.

Reviewed By: ajoulin

Differential Revision: D5265558

fbshipit-source-id: c5c258f2d3b28e6723652c39b3ad6fd613c1d595
  • Loading branch information
cpuhrsch authored and facebook-github-bot committed Jul 17, 2017
1 parent a2cff91 commit 8f03626
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/qmatrix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "qmatrix.h"

#include <assert.h>
#include <cmath>
#include <iostream>

namespace fasttext {
Expand All @@ -20,8 +19,10 @@ QMatrix::QMatrix() : qnorm_(false),

QMatrix::QMatrix(const Matrix& mat, int32_t dsub, bool qnorm)
: qnorm_(qnorm), m_(mat.m_), n_(mat.n_),
codesize_(m_ * std::ceil(n_ / dsub)) {
codes_ = new uint8_t[codesize_];
codesize_(m_ * ((n_ + dsub - 1) / dsub)) {
if (codesize_ > 0) {
codes_ = new uint8_t[codesize_];
}
pq_ = std::unique_ptr<ProductQuantizer>( new ProductQuantizer(n_, dsub));
if (qnorm_) {
norm_codes_ = new uint8_t[m_];
Expand All @@ -31,7 +32,9 @@ QMatrix::QMatrix(const Matrix& mat, int32_t dsub, bool qnorm)
}

QMatrix::~QMatrix() {
if (codesize_) { delete[] codes_; }
if (codesize_ > 0) {
delete[] codes_;
}
if (qnorm_) { delete[] norm_codes_; }
}

Expand Down

0 comments on commit 8f03626

Please sign in to comment.