[education/rkward] rkward/rbackend: Minor cleanups

Thomas Friedrichsmeier null at kde.org
Sun Sep 14 19:26:16 BST 2025


Git commit 7bb5f2bbc74d462ae1b94343449d3e46d232883b by Thomas Friedrichsmeier.
Committed on 09/09/2025 at 16:31.
Pushed by tfry into branch 'master'.

Minor cleanups

M  +9    -33   rkward/rbackend/rkrbackend.cpp
M  +0    -1    rkward/rbackend/rkrbackendprotocol_backend.cpp
M  +6    -7    rkward/rbackend/rkstructuregetter.cpp

https://invent.kde.org/education/rkward/-/commit/7bb5f2bbc74d462ae1b94343449d3e46d232883b

diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index afa96039d..2e3701e66 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -58,7 +58,7 @@ structRstart RK_R_Params;
 ///// i18n
 #include <KLocalizedString>
 #define RK_MSG_DOMAIN "rkward"
-void RK_setupGettext(const QString &locale_dir) {
+static void RK_setupGettext(const QString &locale_dir) {
 	KLocalizedString::setApplicationDomain(RK_MSG_DOMAIN);
 	if (!locale_dir.isEmpty()) {
 		KLocalizedString::addDomainLocaleDir(RK_MSG_DOMAIN, locale_dir);
@@ -143,16 +143,12 @@ void RKRBackend::interruptCommand(int command_id) {
 	}
 }
 
-void clearPendingInterrupt_Worker(void *) {
-	RFn::R_CheckUserInterrupt();
-}
-
 void RKRBackend::clearPendingInterrupt() {
 	RK_TRACE(RBACKEND);
 	if (!RK_IsRInterruptPending()) return;
 	RKRBackend::repl_status.interrupted = false;
 
-	bool passed = RFn::R_ToplevelExec(clearPendingInterrupt_Worker, nullptr);
+	bool passed = RFn::R_ToplevelExec([](void *) { RFn::R_CheckUserInterrupt(); }, nullptr);
 	if (!passed) RK_DEBUG(RBACKEND, DL_DEBUG, "pending interrupt cleared");
 	RK_ASSERT(!passed);
 }
@@ -167,7 +163,7 @@ static void markLastWarningAsErrorMessage() {
 extern SEXP RKWard_RData_Tag;
 
 // ############## R Standard callback overrides BEGIN ####################
-void RKTransmitNextUserCommandChunk(unsigned char *buf, int buflen) {
+static void RKTransmitNextUserCommandChunk(unsigned char *buf, int buflen) {
 	RK_TRACE(RBACKEND);
 
 	RK_ASSERT(RKRBackend::repl_status.user_command_transmitted_up_to <= RKRBackend::repl_status.user_command_buffer.length()); // NOTE: QByteArray::length () does not count the trailing '\0'
@@ -352,7 +348,7 @@ int RReadConsole(const char *prompt, unsigned char *buf, int buflen, int hist) {
 	RKRBackend::this_pointer->handleRequest(&request);
 	if (request.params[QStringLiteral("cancelled")].toBool()) {
 		if (RKRBackend::this_pointer->current_command) RKRBackend::this_pointer->current_command->status |= RCommand::Canceled;
-		RFn::Rf_error("cancelled");
+		RFn::Rf_error("cancelled"); // TODO: probably a memleak, as d'tors (request, params) won't be called
 		RK_ASSERT(false); // should not reach this point.
 	}
 
@@ -519,7 +515,7 @@ void RKRBackend::tryToDoEmergencySave() {
 	}
 }
 
-QStringList charPArrayToQStringList(const char **chars, int count) {
+static QStringList charPArrayToQStringList(const char **chars, int count) {
 	QStringList ret;
 	for (int i = 0; i < count; ++i) {
 		// do we need to do locale conversion, here?
@@ -733,7 +729,6 @@ void RResetConsole() {
 
 // ############## R Standard callback overrides END ####################
 
-SEXP doUpdateLocale();
 // NOTE: stdout_stderr_mutex is recursive to support fork()s, better
 RKRBackend::RKRBackend() : stdout_stderr_mutex() {
 	RK_TRACE(RBACKEND);
@@ -741,7 +736,7 @@ RKRBackend::RKRBackend() : stdout_stderr_mutex() {
 	RK_ASSERT(this_pointer == nullptr);
 	this_pointer = this;
 
-	doUpdateLocale();
+	RKTextCodec::reinit();
 	r_running = false;
 
 	current_command = nullptr;
@@ -786,11 +781,6 @@ void RKRBackend::connectCallbacks() {
 void RKRBackend::setupCallbacks() {
 	RK_TRACE(RBACKEND);
 }
-/*
-SEXP dummyselectlist (SEXP, SEXP, SEXP, SEXP) {
-    qDebug ("got it");
-    return ROb(R_NilValue);
-}*/
 
 void RKRBackend::connectCallbacks() {
 	RK_TRACE(RBACKEND);
@@ -813,8 +803,6 @@ void RKRBackend::connectCallbacks() {
 	// TODO: R devels disabled this for some reason. We set it anyway...
 	ROb(ptr_R_EditFile) = REditFile;
 	//	ROb(ptr_R_EditFiles) = REditFiles;		// undefined reference
-	/*	ROb(ptr_do_selectlist) = dummyselectlist;
-	    ROb(ptr_do_dataviewer) = dummyselectlist;*/
 
 	// these two, we won't override
 	//	ROb(ptr_R_loadhistory) = ... 	// we keep our own history
@@ -832,20 +820,12 @@ SEXP doRCall(SEXP _call, SEXP _args, SEXP _sync, SEXP _nested) {
 
 	RFn::R_CheckUserInterrupt();
 
-	QString call = RKRSupport::SEXPToStringList(_call).value(0);
-	/*	// this is a useful place to sneak in test code for profiling
-	    if (list.value (0) == "testit") {
-	        for (int i = 10000; i >= 1; --i) {
-	            setWarnOption (i);
-	        }
-	        return ROb(R_NilValue);
-	    } */
 	bool sync = RKRSupport::SEXPToInt(_sync);
 	bool nested = RKRSupport::SEXPToInt(_nested);
 	RKRBackend::RequestFlags flags = sync ? (nested ? RKRBackend::SynchronousWithSubcommands : RKRBackend::Synchronous) : RKRBackend::Asynchronous;
 
 	// For now, for simplicity, assume args are always strings, although possibly nested in lists
-	auto ret = RKRBackend::this_pointer->doRCallRequest(call, RKRSupport::SEXPToNestedStrings(_args), flags);
+	auto ret = RKRBackend::this_pointer->doRCallRequest(RKRSupport::SEXPToStringList(_call).value(0), RKRSupport::SEXPToNestedStrings(_args), flags);
 	if (!ret.warning.isEmpty()) RFn::Rf_warning("%s", RKTextCodec::toNative(ret.warning).constData()); // print warnings, first, as errors will cause a stop
 	if (!ret.error.isEmpty()) RFn::Rf_error("%s", RKTextCodec::toNative(ret.error).constData());
 
@@ -900,10 +880,6 @@ SEXP doSimpleBackendCall(SEXP _call) {
 	return ROb(R_NilValue);
 }
 
-void R_CheckStackWrapper(void *) {
-	RFn::R_CheckStack();
-}
-
 SEXP doUpdateLocale() {
 	RK_TRACE(RBACKEND);
 
@@ -1002,12 +978,12 @@ bool RKRBackend::startR() {
 #endif
 
 	RFn::setup_Rmainloop();
-	doUpdateLocale(); // call again, as locale may have been re-initialized during startup
+	RKTextCodec::reinit(); // call again, as locale may have been re-initialized during startup
 
 #ifndef Q_OS_WIN
 	// safety check: If we are beyond the stack boundaries already, we better disable stack checking
 	// this has to come *after* the first setup_Rmainloop ()!
-	Rboolean stack_ok = RFn::R_ToplevelExec(R_CheckStackWrapper, nullptr);
+	Rboolean stack_ok = RFn::R_ToplevelExec([](void *) { RFn::R_CheckStack(); }, nullptr);
 	if (!stack_ok) {
 		RK_DEBUG(RBACKEND, DL_WARNING, "R_CheckStack() failed during initialization. Will disable stack checking and try to re-initialize.");
 		RK_DEBUG(RBACKEND, DL_WARNING, "Whether or not things work after this, *please* submit a bug report.");
diff --git a/rkward/rbackend/rkrbackendprotocol_backend.cpp b/rkward/rbackend/rkrbackendprotocol_backend.cpp
index 3e765d733..b69a4bde7 100644
--- a/rkward/rbackend/rkrbackendprotocol_backend.cpp
+++ b/rkward/rbackend/rkrbackendprotocol_backend.cpp
@@ -29,7 +29,6 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #	include <CoreFoundation/CoreFoundation.h>
 #endif
 
-void RK_setupGettext(const QString &);
 QMutex RK_Debug_Mutex;
 
 void RKDebugMessageOutput(QtMsgType type, const QMessageLogContext &, const QString &msg) {
diff --git a/rkward/rbackend/rkstructuregetter.cpp b/rkward/rbackend/rkstructuregetter.cpp
index b1505b017..531dafddb 100644
--- a/rkward/rbackend/rkstructuregetter.cpp
+++ b/rkward/rbackend/rkstructuregetter.cpp
@@ -112,7 +112,11 @@ void RKStructureGetter::getStructureSafe(SEXP value, const QString &name, int ad
 	args.getter = this;
 	args.nesting_depth = nesting_depth;
 
-	Rboolean ok = RFn::R_ToplevelExec((void (*)(void *))getStructureWrapper, &args);
+	Rboolean ok = RFn::R_ToplevelExec([](void *_data) {
+		auto data = static_cast<GetStructureWorkerArgs *>(_data);
+		data->getter->getStructureWorker(data->toplevel, data->name, data->add_type_flags, data->storage, data->nesting_depth);
+	},
+	                                  &args);
 
 	if (ok != TRUE) {
 		storage->discardData();
@@ -121,12 +125,6 @@ void RKStructureGetter::getStructureSafe(SEXP value, const QString &name, int ad
 	}
 }
 
-void RKStructureGetter::getStructureWrapper(GetStructureWorkerArgs *data) {
-	RK_TRACE(RBACKEND);
-
-	data->getter->getStructureWorker(data->toplevel, data->name, data->add_type_flags, data->storage, data->nesting_depth);
-}
-
 /** Temporarily resolve a promise, usually without keeping its value (unless keep_evalled_promises is set, which it never is, at the time of this writing).
  *  This is useful for peeking into large objects while building the object tree, without permanently using lots of RAM.
  *
@@ -160,6 +158,7 @@ SEXP RKStructureGetter::resolvePromise(SEXP from) {
 }
 
 // TODO: split out some of the large blocks into helper functions, to make this easier to read
+//       review for memory leaks in case of R level errors
 void RKStructureGetter::getStructureWorker(SEXP val, const QString &name, int add_type_flags, RData *storage, int nesting_depth) {
 	RK_TRACE(RBACKEND);
 



More information about the rkward-tracker mailing list