From f142e9a9c6ba493c09a282544da8715b3656648e Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Date: Tue, 20 Jul 2021 07:24:07 +0200
Subject: [PATCH] core: C++-ify membuffer

C-style memory management is a pain and nearly nobody seems to get
it right. Add a C++-version of membuffer that frees the buffer
when it gets out-of-scope. Originally, I was thinking about
conditionally adding a constructor/destructor pair when compiling
with C++. But then decided to create a derived class membufferpp,
because it would be extremely confusing to have behavioral change
when changing a source from from C to C++ or vice-versa.

Also add a comment about the dangers of returned pointer: They
become dangling on changes to the membuffer.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
---
 Subsurface-mobile.pro               |  2 +-
 core/CMakeLists.txt                 |  2 +-
 core/format.cpp                     |  2 --
 core/{membuffer.c => membuffer.cpp} | 12 +++++++++++-
 core/membuffer.h                    | 17 +++++++++++++----
 5 files changed, 26 insertions(+), 9 deletions(-)
 rename core/{membuffer.c => membuffer.cpp} (97%)

diff --git a/Subsurface-mobile.pro b/Subsurface-mobile.pro
index 598e94e4c..2e1b6a959 100644
--- a/Subsurface-mobile.pro
+++ b/Subsurface-mobile.pro
@@ -86,7 +86,7 @@ SOURCES += subsurface-mobile-main.cpp \
 	core/divesite.c \
 	core/equipment.c \
 	core/gas.c \
-	core/membuffer.c \
+	core/membuffer.cpp \
 	core/selection.cpp \
 	core/sha1.c \
 	core/string-format.cpp \
diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt
index c2103ab71..5b356e53f 100644
--- a/core/CMakeLists.txt
+++ b/core/CMakeLists.txt
@@ -122,7 +122,7 @@ set(SUBSURFACE_CORE_LIB_SRCS
 	libdivecomputer.h
 	liquivision.c
 	load-git.c
-	membuffer.c
+	membuffer.cpp
 	membuffer.h
 	metadata.cpp
 	metadata.h
diff --git a/core/format.cpp b/core/format.cpp
index a540f4c21..3da97293a 100644
--- a/core/format.cpp
+++ b/core/format.cpp
@@ -397,7 +397,6 @@ int vasprintf_loc(char **dst, const char *cformat, va_list ap)
 	return utf8.size();
 }
 
-// This function is defined here instead of membuffer.c, because it needs to access QString.
 extern "C" void put_vformat_loc(struct membuffer *b, const char *fmt, va_list args)
 {
 	QByteArray utf8 = vqasprintf_loc(fmt, args).toUtf8();
@@ -408,4 +407,3 @@ extern "C" void put_vformat_loc(struct membuffer *b, const char *fmt, va_list ar
 	memcpy(b->buffer + b->len, data, utf8_size);
 	b->len += utf8_size;
 }
-
diff --git a/core/membuffer.c b/core/membuffer.cpp
similarity index 97%
rename from core/membuffer.c
rename to core/membuffer.cpp
index 3b42797e1..2c0af9f0d 100644
--- a/core/membuffer.c
+++ b/core/membuffer.cpp
@@ -12,6 +12,16 @@
 #include "units.h"
 #include "membuffer.h"
 
+membufferpp::membufferpp()
+	: membuffer{0, 0, nullptr}
+{
+}
+
+membufferpp::~membufferpp()
+{
+	free_buffer(this);
+}
+
 /* Only for internal use */
 static char *detach_buffer(struct membuffer *b)
 {
@@ -66,7 +76,7 @@ void make_room(struct membuffer *b, unsigned int size)
 		char *n;
 		/* round it up to not reallocate all the time.. */
 		needed = needed * 9 / 8 + 1024;
-		n = realloc(b->buffer, needed);
+		n = (char *)realloc(b->buffer, needed);
 		if (!n)
 			oom();
 		b->buffer = n;
diff --git a/core/membuffer.h b/core/membuffer.h
index 6d497371c..821b0cecb 100644
--- a/core/membuffer.h
+++ b/core/membuffer.h
@@ -36,10 +36,6 @@
 #ifndef MEMBUFFER_H
 #define MEMBUFFER_H
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 #include <ctype.h>
 #include <stdio.h>
 #include <stdarg.h>
@@ -50,6 +46,17 @@ struct membuffer {
 	char *buffer;
 };
 
+#ifdef __cplusplus
+
+// In C++ code use this - it automatically frees the buffer, when going out of scope.
+struct membufferpp : public membuffer {
+	membufferpp();
+	~membufferpp();
+};
+
+extern "C" {
+#endif
+
 #ifdef __GNUC__
 #define __printf(x, y) __attribute__((__format__(__printf__, x, y)))
 #else
@@ -64,6 +71,8 @@ extern void put_bytes(struct membuffer *, const char *, int);
 extern void put_string(struct membuffer *, const char *);
 extern void put_quoted(struct membuffer *, const char *, int, int);
 extern void strip_mb(struct membuffer *);
+
+/* The pointer obtained by mb_cstring is invalidated by any modifictation to the membuffer! */
 extern const char *mb_cstring(struct membuffer *);
 extern __printf(2, 0) void put_vformat(struct membuffer *, const char *, va_list);
 extern __printf(2, 0) void put_vformat_loc(struct membuffer *, const char *, va_list);