Skip to content

Conversation

@Noremos
Copy link
Contributor

@Noremos Noremos commented Sep 20, 2025

Postfix for #8738

@dyemanov
Copy link
Member

I'm OK with the correction for master, but I'm not sure we can backport std::string_view into v5. Will you provide a separate patch for v5?

@Noremos
Copy link
Contributor Author

Noremos commented Sep 21, 2025

I'm OK with the correction for master, but I'm not sure we can backport std::string_view into v5. Will you provide a separate patch for v5?

Done: #8748

static const char empty_string[] = "";
if (!string)
static constexpr std::string_view empty_string = "<empty statement>";
if (m_string == nullptr || (m_string_len == 0 && (m_string_len = fb_strlen(m_string)) == 0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this if wih attribution of m_string_len is very confusing.

Copy link
Contributor Author

@Noremos Noremos Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this option suitable (more code, but cleaner execution):

diff --git a/src/jrd/trace/TraceDSQLHelpers.h b/src/jrd/trace/TraceDSQLHelpers.h
index c8c2a230bf..a47d5aacca 100644
--- a/src/jrd/trace/TraceDSQLHelpers.h
+++ b/src/jrd/trace/TraceDSQLHelpers.h
@@ -55,11 +55,17 @@ public:
 
 		m_start_clock = fb_utils::query_performance_counter();
 
-		static constexpr std::string_view empty_string = "<empty statement>";
-		if (m_string == nullptr || (m_string_len == 0 && (m_string_len = fb_strlen(m_string)) == 0))
+		if (m_string == nullptr)
 		{
-			m_string = empty_string.data();
-			m_string_len = empty_string.length();
+			traceEmptyStatement();
+		}
+		else
+		{
+			if (m_string_len == 0)
+				m_string_len = fb_strlen(m_string);
+
+			if (m_string_len == 0)
+				traceEmptyStatement();
 		}
 	}
 
@@ -105,6 +111,13 @@ public:
 	}
 
 private:
+	void traceEmptyStatement()
+	{
+		static constexpr std::string_view empty_string = "<empty statement>";
+		m_string = empty_string.data();
+		m_string_len = empty_string.length();
+	}
+
 	bool m_need_trace;
 	Attachment* m_attachment;
 	jrd_tra* const m_transaction;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this:

if (m_string == nullptr)
	traceEmptyStatement();
else if (m_string_len == 0)
{
	m_string_len = fb_strlen(m_string);
	if (m_string_len == 0)
		traceEmptyStatement();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I applied these changes.

@dyemanov dyemanov merged commit aa92b17 into FirebirdSQL:master Oct 7, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants