anna
/
ocarina
1
0
Fork 0
Code Issues 7 Pull Requests Releases Wiki Activity

database: Store pointers in the database 

Inserting into a vector can sometimes cause the entire vector to
reallocate itself.  The insert() function returns a pointer to the
caller, so this reallocation could invalidate the returned pointer.

This is not what we want.

Instead, store pointers to the data in the vector.  C++ provides a
default copy constructor that can be used to allocate a new item before
inserting.  By doing it this way callers won't have to allocate memory
themselves.  In addition, I will no longer need to keep a valid bit
since we can simply check for a NULL entry in the database.

Signed-off-by: Anna Schumaker <anna@ocarinaproject.net>
This commit is contained in:
Anna Schumaker 2014-03-25 10:57:09 -04:00 committed by Anna Schumaker
parent 38990748bb
commit a9dae134d0
9 changed files with 127 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
DESIGN
View File

@ -199,7 +199,6 @@ Database Entry:
- DatabaseEntry:
class DatabaseEntry {
public:
bool valid;
unsigned int id;
DatabaseEntry();
@ -212,7 +211,7 @@ Database Entry:
- API:
DatabaseEntry :: DatabaseEntry():
Set valid = false and id = 0.
Set id = 0.
const std::string DatabaseEntry :: primary_key();
This function should return a unique string representing this
@ -251,12 +250,6 @@ Database:
uniqueness. This key is used when inserting a new value into the
Database, and will not be updated after.
- Valid bit:
The "valid" bit of a DatabaseEntry is completely managed by the entry's
Database container. It will be set to true when an entry is inserted
and false when deleted. The Database is also in charge of writing the
valid bit to file.
- Id:
The "id" field of a DatabaseEntry is also managed by the entry's
Database container It will be set to the entry's ID when the entry is
@ -266,17 +259,18 @@ Database:
template <class T>
class Database {
private:
std::vector<T> _db;
std::vector<T *> _db;
std::map<const std::string, unsigned int> _keys;
unsigned int _size; /* Number of valid rows */
bool _autosave;
File _file;
public:
typedef std::vector<T>::iterator iterator;
typedef std::vector<T>::const_iterator const_iterator;
typedef std::vector<T *>::iterator iterator;
typedef std::vector<T *>::const_iterator const_iterator;
Database(std::string, bool);
~Database();
void save();
void load();
@ -305,28 +299,32 @@ Database:
data on disk. If the file already exists, read the data into
the backing vector.
Database :: ~Database();
Delete all entries remaining in the database.
void Database :: save();
Save the database to disk.
void Database :: load();
Load the database from disk.
T *Database :: insert(T &item);
T *Database :: insert(T item);
Look up the item in the _keys map.
If we find an item with the same key:
- Return an iterator to the found item.
- Return a pointer to the found item.
Otherwise:
- Use new to allocate memory for a new item.
- Add the new item to the end of the _db.
- Add the new item to _keys.
- Set item.valid = true.
- Set item.id to the index of the new item.
- Set new item.id to the index of the new item.
_ Increment _size.
- If autosave == true: save().
- Return an iterator to the new item.
- Return a pointer to the new item.
unsigned int Database :: remove();
unsigned int Database :: remove(unsigned int id);
- Remove the item from the _keys map.
- Set item.valid = false.
- Delete _db[id]
- Set _db[id] = NULL
- If autosave == true: save().
- Decrement _size.
@ -351,10 +349,10 @@ Database:
if there are no valid entries left.
T *Database :: at(unsigned int i);
If _db[i].valid == true:
Return a pointer to _db[i];
If i is beyond the end of the container:
Return NULL
Otherwise:
Return NULL;
Return _db[i];
T *Database :: find(const std::string &key);
If key is in the _keys map:

8
include/database.h
View File

@ -13,10 +13,10 @@
class DatabaseEntry {
public:
bool valid;
unsigned int id;
DatabaseEntry();
virtual ~DatabaseEntry() = 0;
virtual const std::string primary_key() = 0;
virtual void write(File &) = 0;
virtual void read(File &) = 0;
@ -27,15 +27,15 @@ public:
template <class T>
class Database {
private:
std::vector<T> _db;
std::vector<T *> _db;
std::map<const std::string, unsigned int> _keys;
unsigned int _size;
bool _autosave;
File _file;
public:
typedef typename std::vector<T>::iterator iterator;
typedef typename std::vector<T>::const_iterator const_iterator;
typedef typename std::vector<T *>::iterator iterator;
typedef typename std::vector<T *>::const_iterator const_iterator;
Database(std::string, bool);
~Database();

56
include/database.hpp
View File

@ -18,6 +18,9 @@ Database<T> :: Database(std::string filepath, bool autosave)
template <class T>
Database<T> :: ~Database()
{
iterator it;
for (it = begin(); it != end(); it = next(it))
delete (*it);
}
template <class T>
@ -26,12 +29,13 @@ void Database<T> :: save()
if (_file.open(OPEN_WRITE) == false)
return;
_file << _db.size() << std::endl;
_file << actual_size() << std::endl;
for (unsigned int i = 0; i < _db.size(); i++) {
_file << _db[i].valid;
if (_db[i].valid == true) {
_file << " ";
_db[i].write(_file);
if (_db[i] == NULL)
_file << false;
else {
_file << true << " ";
_db[i]->write(_file);
}
_file << std::endl;
}
@ -50,6 +54,7 @@ template <class T>
void Database<T> :: load()
{
unsigned int db_size;
bool valid;
if (_file.exists() == false)
return;
@ -59,11 +64,14 @@ void Database<T> :: load()
_file >> db_size;
_db.resize(db_size);
for (unsigned int i = 0; i < db_size; i++) {
_file >> _db[i].valid;
if (_db[i].valid == true) {
_db[i].read(_file);
_db[i].id = i;
_keys.insert(std::pair<std::string, unsigned int>(_db[i].primary_key(), i));
_file >> valid;
if (valid == false)
_db[i] = NULL;
else {
_db[i] = new T;
_db[i]->read(_file);
_db[i]->id = i;
_keys[_db[i]->primary_key()] = i;
_size++;
}
}
@ -79,14 +87,14 @@ T *Database<T> :: insert(T val)
if (t != NULL)
return t;
iterator it = _db.insert(_db.end(), val);
it->valid = true;
it->id = it - _db.begin();
t = new T(val);
_db.push_back(t);
t->id = actual_size() - 1;
_keys[it->primary_key()] = it->id;
_keys[t->primary_key()] = t->id;
_size++;
autosave();
return &_db[it->id];
return t;
}
template <class T>
@ -94,10 +102,11 @@ void Database<T> :: remove(unsigned int id)
{
if (id >= actual_size())
return;
if (_db[id].valid == false)
if (_db[id] == NULL)
return;
_keys.erase(_db[id].primary_key());
_db[id].valid = false;
_keys.erase(_db[id]->primary_key());
delete _db[id];
_db[id] = NULL;
_size--;
autosave();
}
@ -121,7 +130,7 @@ typename Database<T>::iterator Database<T> :: begin()
return end();
iterator it = _db.begin();
if ( (*it).valid == true )
if ( (*it) != NULL )
return it;
return next(it);
}
@ -137,7 +146,7 @@ typename Database<T>::iterator Database<T> :: next(iterator &it)
{
do {
it++;
} while ((it != end()) && ((*it).valid == false));
} while ((it != end()) && (*it) == NULL);
return it;
}
@ -146,10 +155,7 @@ T *Database<T> :: at(unsigned int id)
{
if (id >= actual_size())
return NULL;
if (_db[id].valid == false)
return NULL;
return &_db[id];
return _db[id];
}
template <class T>
@ -159,6 +165,6 @@ T *Database<T> :: find(const std::string &key)
it = _keys.find(key);
if (it == _keys.end())
return NULL;
return &_db[it->second];
return _db[it->second];
}
#endif /* OCARINA_DATABASE_HPP */

4
lib/database.cpp
View File

@ -5,6 +5,8 @@
DatabaseEntry :: DatabaseEntry()
: valid(false), id(0)
: id(0)
{
}
DatabaseEntry :: ~DatabaseEntry() {}

33
lib/library.cpp
View File

@ -282,9 +282,6 @@ static void read_tags(unsigned int lib_id, const std :: string &path)
track_id = track_db.insert(library :: Track(tag, audio, lib_id,
artist_id, album_id, genre_id, path))->id;
if (track_db.at(track_id)->valid == false)
return;
library_db.at(lib_id)->size++;
filter::add(artist_db.at(artist_id)->name, track_id);
@ -356,10 +353,10 @@ static void do_validate_library(unsigned int &lib_id)
Database<library :: Track>::iterator it;
for (it = track_db.begin(); it != track_db.end(); it = track_db.next(it)) {
if ((*it).library_id != lib_id)
if ((*it)->library_id != lib_id)
continue;
path = it->primary_key();
path = (*it)->primary_key();
if (g_file_test(path.c_str(), G_FILE_TEST_EXISTS) == false) {
dprint("Removing file: %s\n", path.c_str());
track_db.remove(it - track_db.begin());
@ -469,17 +466,17 @@ void library :: init()
Database<Track>::iterator it;
for (it = track_db.begin(); it != track_db.end(); it = track_db.next(it)) {
filter::add(artist_db.at((*it).artist_id)->name, it->id);
filter::add(album_db.at((*it).album_id)->name, it->id);
filter::add((*it).title, it->id);
filter::add(artist_db.at((*it)->artist_id)->name, (*it)->id);
filter::add(album_db.at((*it)->album_id)->name, (*it)->id);
filter::add((*it)->title, (*it)->id);
if (library_db.at((*it).library_id)->enabled)
get_callbacks()->on_library_track_add(it->id);
if (library_db.at((*it)->library_id)->enabled)
get_callbacks()->on_library_track_add((*it)->id);
}
Database<Library>::iterator l_it;
for (l_it = library_db.begin(); l_it != library_db.end(); l_it = library_db.next(l_it))
get_callbacks()->on_library_add(l_it->id, &(*library_db.at(l_it->id)));
get_callbacks()->on_library_add((*l_it)->id, &(*library_db.at((*l_it)->id)));
}
void library :: add_path(const std::string &dir)
@ -502,9 +499,9 @@ void library :: del_path(unsigned int id)
Database<Track>::iterator it;
for (it = track_db.begin(); it != track_db.end(); it = track_db.next(it)) {
if ((*it).library_id == id) {
get_callbacks()->on_library_track_del(it->id);
track_db.remove(it->id);
if ((*it)->library_id == id) {
get_callbacks()->on_library_track_del((*it)->id);
track_db.remove((*it)->id);
}
}
@ -518,7 +515,7 @@ void library :: update_path(unsigned int id)
{
if (id > library_db.size())
return;
if (library_db.at(id)->valid == false)
if (library_db.at(id) == NULL)
return;
do_update_library(id);
}
@ -540,7 +537,7 @@ void library :: set_enabled(unsigned int id, bool enabled)
library_db.save();
for (it = track_db.begin(); it != track_db.end(); it = track_db.next(it)) {
if ((*it).library_id == id) {
if ((*it)->library_id == id) {
t = it - track_db.begin();
if (enabled)
get_callbacks()->on_library_track_add(t);
@ -556,7 +553,7 @@ void library :: lookup(unsigned int id, library :: Song *song)
throw -E_EXIST;
song->track = &(*track_db.at(id));
if (song->track->valid == false)
if (song->track == NULL)
throw -E_EXIST;
song->track_id = id;
@ -570,7 +567,7 @@ library :: Library *library :: lookup_path(unsigned int id)
{
if (id >= library_db.actual_size())
throw -E_EXIST;
if (library_db.at(id)->valid == false)
if (library_db.at(id) == NULL)
throw -E_EXIST;
return &(*library_db.at(id));
}

4
lib/tags.cpp
View File

@ -221,8 +221,8 @@ void tagdb :: remove_library(unsigned int library_id)
{
Database<Track>::iterator it;
for (it = track_db.begin(); it != track_db.end(); it++) {
if (it->library->id == library_id)
track_db.remove(it->id);
if ((*it)->library->id == library_id)
track_db.remove((*it)->id);
}
library_db.remove(library_id);
}

4
tests/db_entry
View File

@ -10,7 +10,7 @@ function test_entry
function test_print
{
test_entry $1 $2 "Value: $1 Key: $2 Valid: 0 Id: 0"
test_entry $1 $2 "Value: $1 Key: $2 Id: 0"
}
function test_primary_key
@ -30,7 +30,7 @@ function test_read
rm $DATA_DIR/db_entry.txt 2>/dev/null || true
echo 0 > $DATA_DIR/db_entry.txt
echo $1 $2 >> $DATA_DIR/db_entry.txt
test_entry "-r $1" $2 "Value: $1 Key: $2 Valid: 0 Id: 0"
test_entry "-r $1" $2 "Value: $1 Key: $2 Id: 0"
}

62
tests/src/database.cpp
View File

@ -84,9 +84,24 @@ int main(int argc, char **argv)
/**
* 1: Test insertion
*/
IntEntry *one, *three;
for (unsigned int i = 0; i < n; i++) {
if (db.insert(IntEntry(i))->id != i)
IntEntry *it = db.insert(IntEntry(i));
if (it == NULL)
test_results(false, __LINE__);
if (it->id != i)
test_results(false, __LINE__);
if (it->val != i)
test_results(false, __LINE__);
/*
* Pointers should still be valid after more insertions.
* These will be checked later
*/
if (i == 1)
one = it;
if (i == 3)
three = it;
}
test_results(true, __LINE__);
size += n;
@ -103,7 +118,12 @@ int main(int argc, char **argv)
* 3: Test inserting ... again.
*/
for (unsigned int i = 0; i < n; i++) {
if (db.insert(IntEntry(i))->id != i)
IntEntry *it = db.insert(IntEntry(i));
if (it->id != i)
test_results(false, __LINE__);
if (i == 1 && it != one)
test_results(false, __LINE__);
if (i == 3 && it != three)
test_results(false, __LINE__);
}
test_results(true, __LINE__);
@ -132,7 +152,7 @@ int main(int argc, char **argv)
/**
* 6: Test that removing again doesn't change anything
*/
for (unsigned int i = 0; i < n; i+=2)
for (unsigned int i = 0; i < n + 10; i+=2)
db.remove(i);
test_size(db, size, actual, __LINE__);
@ -143,9 +163,11 @@ int main(int argc, char **argv)
Database<IntEntry>::iterator it;
unsigned int index = 1;
for (it = db.begin(); it != db.end(); it = db.next(it)) {
if ((*it).val != index)
if ((*it) == NULL)
test_results(false, __LINE__);
if ((*it).id != index)
if ((*it)->val != index)
test_results(false, __LINE__);
if ((*it)->id != index)
test_results(false, __LINE__);
index+=2;
};
@ -157,9 +179,23 @@ int main(int argc, char **argv)
*/
for (unsigned int i = 0; i < n + 10; i++) {
IntEntry *it = db.at(i);
if (((i % 2) == 0) && (it != NULL))
if (i >= n) {
if (it != NULL)
test_results(false, __LINE__);
continue;
} else if (i % 2 == 0) {
if (it != NULL)
test_results(false, __LINE__);
} else if (it == NULL)
test_results(false, __LINE__);
if ((i >= n) && (it != NULL))
/*
* Remember those pointers we stored earlier?
* The should still be valid now.
*/
if (i == 1 && it != one)
test_results(false, __LINE__);
if (i == 3 && it != three)
test_results(false, __LINE__);
}
test_results(true, __LINE__);
@ -169,7 +205,7 @@ int main(int argc, char **argv)
* 9. Test inserting once again
*/
for (unsigned int i = 0; i < n; i++) {
index = db.insert(i)->id;
index = db.insert(IntEntry(i))->id;
if ((i % 2) == 0) {
size++;
actual++;
@ -210,12 +246,14 @@ int main(int argc, char **argv)
*/
Database<IntEntry>::iterator it2 = db2.begin();
for (it = db.begin(); it != db.end(); it++) {
if (it->valid != it2->valid)
if ((*it) == NULL && (*it2) != NULL)
test_results(false, __LINE__);
if (it->valid == true) {
if (it->val != it2->val)
if ((*it) != NULL && (*it2) == NULL)
test_results(false, __LINE__);
if ((*it) != NULL) {
if ((*it)->val != (*it2)->val)
test_results(false, __LINE__);
if (it->id != it2->id)
if ((*it)->id != (*it2)->id)
test_results(false, __LINE__);
}
it2++;

2
tests/src/db_entry.cpp
View File

@ -48,7 +48,7 @@ void IntEntry :: read(File &f)
void IntEntry :: print()
{
:: print("Value: %u Key: %s Valid: %d Id: %u\n", val, key.c_str(), valid, id);
:: print("Value: %u Key: %s Id: %u\n", val, key.c_str(), id);
}