core: turn a memblock in the parser to std::string

This avoid memory-management troubles. Had to convert a few
of the parsers (cochran, datatrak, liquivision) to C++.
Also had to convert libdivecomputer.c. This was less
painful than expected.

std::string is used because parts of the code assumes
that the data is null terminated after the last character
of the data. std::string does precisely that.

One disadvantage is that std::string clears its memory
when resizing / initializing. Thus we read the file onto
freshly cleared data, which some might thing is a
performance regression. Until someone shows me that this
matters, I don't care.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2024-03-01 13:09:20 +01:00 committed by Michael Keller
parent 2f4dbf1848
commit cf7c54bd56
12 changed files with 233 additions and 285 deletions

View file

@ -31,61 +31,47 @@
#define O_BINARY 0
#endif
extern "C" int readfile(const char *filename, struct memblock *mem)
std::pair<std::string, int> readfile(const char *filename)
{
int ret, fd;
struct stat st;
char *buf;
mem->buffer = NULL;
mem->size = 0;
std::string res;
fd = subsurface_open(filename, O_RDONLY | O_BINARY, 0);
if (fd < 0)
return fd;
return std::make_pair(res, fd);
ret = fstat(fd, &st);
if (ret < 0)
goto out;
ret = -EINVAL;
return std::make_pair(res, ret);
if (!S_ISREG(st.st_mode))
goto out;
ret = 0;
return std::make_pair(res, -EINVAL);
if (!st.st_size)
goto out;
buf = (char *)malloc(st.st_size + 1);
ret = -1;
errno = ENOMEM;
if (!buf)
goto out;
mem->buffer = buf;
mem->size = st.st_size;
ret = read(fd, buf, mem->size);
return std::make_pair(res, 0);
// Sadly, this 0-initializes the string, just before overwriting it.
// However, we use std::string, because that automatically 0-terminates
// the data and the code expects that.
res.resize(st.st_size);
ret = read(fd, res.data(), res.size());
if (ret < 0)
goto free;
buf[ret] = 0;
if (ret == (int)mem->size) // converting to int loses a bit but size will never be that big
goto out;
errno = EIO;
ret = -1;
free:
free(mem->buffer);
mem->buffer = NULL;
mem->size = 0;
out:
close(fd);
return ret;
return std::make_pair(res, ret);
// converting to int loses a bit but size will never be that big
if (ret == (int)res.size()) {
return std::make_pair(res, ret);
} else {
errno = EIO;
return std::make_pair(res, -1);
}
}
static void zip_read(struct zip_file *file, const char *filename, struct divelog *log)
{
int size = 1024, n, read = 0;
std::vector<char> mem(size);
std::vector<char> mem(size + 1);
while ((n = zip_fread(file, mem.data() + read, size - read)) > 0) {
read += n;
size = read * 3 / 2;
mem.resize(size);
mem.resize(size + 1);
}
mem[read] = 0;
(void) parse_xml_buffer(filename, mem.data(), read, log, NULL);
@ -123,7 +109,7 @@ static int db_test_func(void *, int, char **data, char **)
return *data[0] == '0';
}
static int try_to_open_db(const char *filename, struct memblock *mem, struct divelog *log)
static int try_to_open_db(const char *filename, std::string &mem, struct divelog *log)
{
sqlite3 *handle;
char dm4_test[] = "select count(*) from sqlite_master where type='table' and name='Dive' and sql like '%ProfileBlob%'";
@ -145,7 +131,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Suunto DM5 database format */
retval = sqlite3_exec(handle, dm5_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_dm5_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_dm5_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -153,7 +139,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Suunto DM4 database format */
retval = sqlite3_exec(handle, dm4_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_dm4_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_dm4_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -161,7 +147,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Shearwater database format */
retval = sqlite3_exec(handle, shearwater_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_shearwater_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_shearwater_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -169,7 +155,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Shearwater cloud database format */
retval = sqlite3_exec(handle, shearwater_cloud_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_shearwater_cloud_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_shearwater_cloud_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -177,7 +163,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Atomic Cobalt database format */
retval = sqlite3_exec(handle, cobalt_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_cobalt_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_cobalt_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -185,7 +171,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Divinglog database format */
retval = sqlite3_exec(handle, divinglog_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_divinglog_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_divinglog_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -193,7 +179,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
/* Testing if DB schema resembles Seac database format */
retval = sqlite3_exec(handle, seacsync_test, &db_test_func, 0, NULL);
if (!retval) {
retval = parse_seac_buffer(handle, filename, (char *)mem->buffer, mem->size, log);
retval = parse_seac_buffer(handle, filename, mem.data(), mem.size(), log);
sqlite3_close(handle);
return retval;
}
@ -220,7 +206,7 @@ static int try_to_open_db(const char *filename, struct memblock *mem, struct div
*
* Followed by the data values (all comma-separated, all one long line).
*/
static int open_by_filename(const char *filename, const char *fmt, struct memblock *mem, struct divelog *log)
static int open_by_filename(const char *filename, const char *fmt, std::string &mem, struct divelog *log)
{
// hack to be able to provide a comment for the translated string
static struct { const char *s; const char *comment; } csv_warning =
@ -251,17 +237,17 @@ static int open_by_filename(const char *filename, const char *fmt, struct memblo
return 0;
}
static int parse_file_buffer(const char *filename, struct memblock *mem, struct divelog *log)
static int parse_file_buffer(const char *filename, std::string &mem, struct divelog *log)
{
int ret;
const char *fmt = strrchr(filename, '.');
if (fmt && (ret = open_by_filename(filename, fmt + 1, mem, log)) != 0)
return ret;
if (!mem->size || !mem->buffer)
if (mem.empty())
return report_error("Out of memory parsing file %s\n", filename);
return parse_xml_buffer(filename, (char *)mem->buffer, mem->size, log, NULL);
return parse_xml_buffer(filename, mem.data(), mem.size(), log, NULL);
}
extern "C" bool remote_repo_uptodate(const char *filename, struct git_info *info)
@ -284,9 +270,7 @@ extern "C" bool remote_repo_uptodate(const char *filename, struct git_info *info
extern "C" int parse_file(const char *filename, struct divelog *log)
{
struct git_info info;
struct memblock mem;
const char *fmt;
int ret;
if (is_git_repository(filename, &info)) {
if (!open_git_repository(&info)) {
@ -300,60 +284,49 @@ extern "C" int parse_file(const char *filename, struct divelog *log)
}
}
ret = git_load_dives(&info, log);
int ret = git_load_dives(&info, log);
cleanup_git_info(&info);
return ret;
}
if ((ret = readfile(filename, &mem)) < 0) {
auto [mem, err] = readfile(filename);
if (err < 0) {
/* we don't want to display an error if this was the default file */
if (same_string(filename, prefs.default_filename))
return 0;
return report_error(translate("gettextFromC", "Failed to read '%s'"), filename);
} else if (ret == 0) {
} else if (err == 0) {
return report_error(translate("gettextFromC", "Empty file '%s'"), filename);
}
fmt = strrchr(filename, '.');
if (fmt && (!strcasecmp(fmt + 1, "DB") || !strcasecmp(fmt + 1, "BAK") || !strcasecmp(fmt + 1, "SQL"))) {
if (!try_to_open_db(filename, &mem, log)) {
free(mem.buffer);
if (!try_to_open_db(filename, mem, log))
return 0;
}
}
/* Divesoft Freedom */
if (fmt && (!strcasecmp(fmt + 1, "DLF"))) {
ret = parse_dlf_buffer((unsigned char *)mem.buffer, mem.size, log);
free(mem.buffer);
return ret;
}
if (fmt && (!strcasecmp(fmt + 1, "DLF")))
return parse_dlf_buffer((unsigned char *)mem.data(), mem.size(), log);
/* DataTrak/Wlog */
if (fmt && !strcasecmp(fmt + 1, "LOG")) {
struct memblock wl_mem;
const char *t = strrchr(filename, '.');
std::string wl_name = std::string(filename, t - filename) + ".add";
if((ret = readfile(wl_name.c_str(), &wl_mem)) < 0) {
auto [wl_mem, err] = readfile(wl_name.c_str());
if (err < 0) {
fprintf(stderr, "No file %s found. No WLog extensions.\n", wl_name.c_str());
ret = datatrak_import(&mem, NULL, log);
} else {
ret = datatrak_import(&mem, &wl_mem, log);
free(wl_mem.buffer);
wl_mem.clear();
}
free(mem.buffer);
return ret;
return datatrak_import(mem, wl_mem, log);
}
/* OSTCtools */
if (fmt && (!strcasecmp(fmt + 1, "DIVE"))) {
free(mem.buffer);
ostctools_import(filename, log);
return 0;
}
ret = parse_file_buffer(filename, &mem, log);
free(mem.buffer);
return ret;
return parse_file_buffer(filename, mem, log);
}