lib: add diagnostic substitute class for ec_error_t

Similar to errno_t, make ec_error_t a struct that prohibits use of bool conversions to detect misuses of ec_error_t return values.
grommunio · Feb 15, 2025 · 4e72f0d · 4e72f0d
1 parent b0e0da6
commit 4e72f0d
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 22 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -40,4 +40,6 @@ jobs:
         ./qconf
         LD_LIBRARY_PATH=/usr/local/lib make "-j$(nproc)"
         LD_LIBRARY_PATH=/usr/local/lib make install DESTDIR="$PWD/rt" && rm -Rf rt
+        make clean
+        LD_LIBRARY_PATH=/usr/local/lib make "-j$(nproc)" CPPFLAGS=-DCOMPILE_DIAG
         make distclean
diff --git a/exch/zcore/common_util.cpp b/exch/zcore/common_util.cpp
@@ -1215,7 +1215,9 @@ static ec_error_t cu_rcpt_to_list(eid_t message_id, const TPROPVAL_ARRAY &props,
 	           g_org_name, cu_id2user, es_result);
 	if (ret == ecSuccess)
 		list.emplace_back(std::move(es_result));
-	return ret == ecNullObject || ret == ecUnknownUser ? ecInvalidRecips : ret;
+	if (ret == ecNullObject || ret == ecUnknownUser)
+		return ecInvalidRecips;
+	return ret;
 } catch (const std::bad_alloc &) {
 	mlog(LV_ERR, "E-1122: ENOMEM");
 	return ecServerOOM;
@@ -1361,7 +1363,7 @@ void common_util_notify_receipt(const char *username, int type,
 	auto ret = cu_send_vmail(std::move(imail), g_smtp_url.c_str(),
 	           username, rcpt_list);
 	if (ret != ecSuccess)
-		mlog(LV_ERR, "E-1193: cu_send_mail: %xh", ret);
+		mlog(LV_ERR, "E-1193: cu_send_mail: %xh", static_cast<unsigned int>(ret));
 } catch (const std::bad_alloc &) {
 	mlog(LV_ERR, "E-2038: ENOMEM");
 }

diff --git a/exch/zcore/object_tree.cpp b/exch/zcore/object_tree.cpp
@@ -263,7 +263,7 @@ std::unique_ptr<OBJECT_TREE> object_tree_create(const char *maildir)
 	if (prootobj == nullptr)
 		return NULL;
 	auto handle = pobjtree->add_object_handle(-1, {zs_objtype::root, std::move(prootobj)});
-	if (zh_error(handle))
+	if (zh_is_error(handle))
 		return nullptr;
 	return pobjtree;
 }

diff --git a/exch/zcore/object_tree.hpp b/exch/zcore/object_tree.hpp
@@ -8,7 +8,14 @@
 
 static inline ec_error_t zh_error(uint32_t h)
 {
-	return h < 0x80000000 ? ecSuccess : static_cast<ec_error_t>(h);
+	if (h >= 0x80000000)
+		return static_cast<ec_error_t>(h);
+	return ecSuccess;
+}
+
+static inline bool zh_is_error(uint32_t h)
+{
+	return h >= 0x80000000;
 }
 
 struct object_node {

diff --git a/exch/zcore/rpc_parser.cpp b/exch/zcore/rpc_parser.cpp
@@ -88,12 +88,12 @@ static int rpc_parser_dispatch(const zcreq *q0, std::unique_ptr<zcresp> &r0) try
 	r0->call_id = q0->call_id;
 	if (g_zrpc_debug == 0)
 		return DISPATCH_TRUE;
-	if (r0->result != 0 || g_zrpc_debug == 2) {
+	if (r0->result != ecSuccess || g_zrpc_debug == 2) {
 		auto info = zs_query_session(dbg_hsession);
 		mlog(LV_DEBUG, "ZRPC %s %5luµs %8xh %s",
 		        info != nullptr ? info->username.c_str() : "<>",
 		        static_cast<unsigned long>(std::chrono::duration_cast<std::chrono::microseconds>(tend - tstart).count()),
-		        r0->result, zcore_rpc_idtoname(q0->call_id));
+		        static_cast<unsigned int>(r0->result), zcore_rpc_idtoname(q0->call_id));
 	}
 	return DISPATCH_TRUE;
 } catch (const std::bad_alloc &) {

diff --git a/exch/zcore/zserver.cpp b/exch/zcore/zserver.cpp
@@ -842,7 +842,7 @@ ec_error_t zs_openentry(GUID hsession, BINARY entryid,
 		    &b_private, &account_id, &folder_id))
 			break;
 		auto handle = pinfo->ptree->get_store_handle(b_private, account_id);
-		if (zh_error(handle))
+		if (zh_is_error(handle))
 			return zh_error(handle);
 		pinfo.reset();
 		return zs_openstoreentry(hsession,
@@ -854,7 +854,7 @@ ec_error_t zs_openentry(GUID hsession, BINARY entryid,
 		    &b_private, &account_id, &folder_id, &message_id))
 			break;
 		auto handle = pinfo->ptree->get_store_handle(b_private, account_id);
-		if (zh_error(handle))
+		if (zh_is_error(handle))
 			return zh_error(handle);
 		pinfo.reset();
 		return zs_openstoreentry(hsession,
@@ -1030,7 +1030,7 @@ ec_error_t zs_openstoreentry(GUID hsession, uint32_t hobject, BINARY entryid,
 		if (pmessage == nullptr)
 			return ecError;
 		*phobject = pinfo->ptree->add_object_handle(hobject, {zs_objtype::message, std::move(pmessage)});
-		if (zh_error(*phobject))
+		if (zh_is_error(*phobject))
 			return zh_error(*phobject);
 		*pmapi_type = zs_objtype::message;
 	} else {
@@ -1084,7 +1084,7 @@ ec_error_t zs_openstoreentry(GUID hsession, uint32_t hobject, BINARY entryid,
 		if (pfolder == nullptr)
 			return ecError;
 		*phobject = pinfo->ptree->add_object_handle(hobject, {zs_objtype::folder, std::move(pfolder)});
-		if (zh_error(*phobject))
+		if (zh_is_error(*phobject))
 			return zh_error(*phobject);
 		*pmapi_type = zs_objtype::folder;
 	}
@@ -1126,7 +1126,7 @@ ec_error_t zs_openabentry(GUID hsession,
 			return ecError;
 		*pmapi_type = zs_objtype::abcont;
 		*phobject = pinfo->ptree->add_object_handle(ROOT_HANDLE, {*pmapi_type, std::move(contobj)});
-		if (zh_error(*phobject))
+		if (zh_is_error(*phobject))
 			return zh_error(*phobject);
 		return ecSuccess;
 	}
@@ -1237,7 +1237,7 @@ static ec_error_t zs_openab_emsab(USER_INFO_REF &&pinfo, BINARY entryid,
 	} else {
 		return ecInvalidParam;
 	}
-	if (zh_error(*phobject))
+	if (zh_is_error(*phobject))
 		return zh_error(*phobject);
 	return ecSuccess;
 }
@@ -1639,7 +1639,7 @@ ec_error_t zs_createmessage(GUID hsession,
 	auto folder_id = pfolder->folder_id;
 	auto pstore = pfolder->pstore;
 	auto hstore = pinfo->ptree->get_store_handle(pstore->b_private, pstore->account_id);
-	if (zh_error(hstore))
+	if (zh_is_error(hstore))
 		return zh_error(hstore);
 	if (!pstore->owner_mode()) {
 		if (!exmdb_client->get_folder_perm(pstore->get_dir(),
@@ -2142,7 +2142,7 @@ ec_error_t zs_createfolder(GUID hsession, uint32_t hparent_folder,
 			because the caller normally will not keep the
 			handle of parent folder */
 		auto hstore = pinfo->ptree->get_store_handle(TRUE, pstore->account_id);
-		if (zh_error(hstore))
+		if (zh_is_error(hstore))
 			return zh_error(hstore);
 		*phobject = pinfo->ptree->add_object_handle(hstore, {zs_objtype::folder, std::move(pfolder)});
 	} else {
@@ -3826,7 +3826,7 @@ ec_error_t zs_openembedded(GUID hsession,
 		return ecNotSupported;
 	auto pstore = pattachment->get_store();
 	auto hstore = pinfo->ptree->get_store_handle(pstore->b_private, pstore->account_id);
-	if (zh_error(hstore))
+	if (zh_is_error(hstore))
 		return zh_error(hstore);
 	auto b_writable = pattachment->writable();
 	auto tag_access = pattachment->get_tag_access();
@@ -4073,7 +4073,7 @@ ec_error_t zs_hierarchysync(GUID hsession, uint32_t hfolder, uint32_t *phobject)
 		return ecNotSupported;
 	auto pstore = pfolder->pstore;
 	auto hstore = pinfo->ptree->get_store_handle(pstore->b_private, pstore->account_id);
-	if (zh_error(hstore))
+	if (zh_is_error(hstore))
 		return zh_error(hstore);
 	auto pctx = icsdownctx_object::create(pfolder, SYNC_TYPE_HIERARCHY);
 	if (pctx == nullptr)
@@ -4095,7 +4095,7 @@ ec_error_t zs_contentsync(GUID hsession, uint32_t hfolder, uint32_t *phobject)
 		return ecNotSupported;
 	auto pstore = pfolder->pstore;
 	auto hstore = pinfo->ptree->get_store_handle(pstore->b_private, pstore->account_id);
-	if (zh_error(hstore))
+	if (zh_is_error(hstore))
 		return zh_error(hstore);
 	auto pctx = icsdownctx_object::create(pfolder, SYNC_TYPE_CONTENTS);
 	if (pctx == nullptr)
@@ -4229,7 +4229,7 @@ ec_error_t zs_hierarchyimport(GUID hsession,
 		return ecNotSupported;
 	auto pstore = pfolder->pstore;
 	auto hstore = pinfo->ptree->get_store_handle(pstore->b_private, pstore->account_id);
-	if (zh_error(hstore))
+	if (zh_is_error(hstore))
 		return zh_error(hstore);
 	auto pctx = icsupctx_object::create(pfolder, SYNC_TYPE_HIERARCHY);
 	if (pctx == nullptr)
@@ -4251,7 +4251,7 @@ ec_error_t zs_contentimport(GUID hsession, uint32_t hfolder, uint32_t *phobject)
 		return ecNotSupported;
 	auto pstore = pfolder->pstore;
 	auto hstore = pinfo->ptree->get_store_handle(pstore->b_private, pstore->account_id);
-	if (zh_error(hstore))
+	if (zh_is_error(hstore))
 		return zh_error(hstore);
 	auto pctx = icsupctx_object::create(pfolder, SYNC_TYPE_CONTENTS);
 	if (pctx == nullptr)
@@ -5152,7 +5152,7 @@ ec_error_t zs_linkmessage(GUID hsession,
 	if (pinfo == nullptr)
 		return ecError;
 	auto handle = pinfo->ptree->get_store_handle(b_private, account_id);
-	if (zh_error(handle))
+	if (zh_is_error(handle))
 		return zh_error(handle);
 	if (pinfo->user_id < 0)
 		return ecAccessDenied;

diff --git a/include/gromox/mapierr.hpp b/include/gromox/mapierr.hpp
@@ -14,7 +14,12 @@
  *
  * -- Changes here should be reflected in lib/errno.cpp.
  */
-enum ec_error_t {
+#ifdef COMPILE_DIAG
+enum ec_error_t_ll
+#else
+enum ec_error_t
+#endif
+{
 	ecSuccess = 0, // ecNone
 	MAPI_E_UNBINDSUCCESS = 1, /* NSPI */
 	// MAPI_E_USER_ABORT = 0x1,
@@ -429,3 +434,19 @@ enum ec_error_t {
 	ecZNullObject = 0xfffffc00,
 	ecZOutOfHandles = 0xfffffc04,
 };
+
+#ifdef COMPILE_DIAG
+struct ec_error_t {
+	constexpr ec_error_t() = default;
+	constexpr ec_error_t(uint32_t x) : m_value(static_cast<ec_error_t_ll>(x)) {}
+	constexpr ec_error_t(ec_error_t_ll x) : m_value(x) {}
+	constexpr ec_error_t(int) = delete;
+	constexpr bool operator==(ec_error_t_ll x) const { return m_value == x; }
+	constexpr bool operator!=(ec_error_t_ll x) const { return m_value != x; }
+	constexpr operator bool() const = delete;
+	constexpr void operator!() const = delete;
+	constexpr operator uint32_t() const { return m_value; }
+	private:
+	ec_error_t_ll m_value = ecSuccess;
+};
+#endif
diff --git a/lib/rfbl.cpp b/lib/rfbl.cpp
@@ -194,7 +194,12 @@ const char *mapi_errname_r(unsigned int e, char *b, size_t bz)
 const char *mapi_strerror(ec_error_t e)
 {
 	// STG = storage
-	switch (e) {
+#ifdef COMPILE_DIAG
+	switch (static_cast<uint32_t>(e))
+#else
+	switch (e)
+#endif
+{
 #define E(v, s) case v: return s;
 	E(ecSuccess, "The operation succeeded")
 	E(ecUnknownUser, "User is unknown to the system")