Skip to content

Commit 02bb7fb

Browse files
CromFrCromFr
committed
cplugin: make GetString return the result to avoid strcpy when possible
1 parent c6b6352 commit 02bb7fb

File tree

5 files changed

+41
-20
lines changed

5 files changed

+41
-20
lines changed

‎include/nwnx_cplugin.h

+10-8
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,16 @@ __declspec(dllexport) void __cdecl NWNXCPlugin_SetFloat(
163163
/// @param sFunction NWScript function argument. Null-terminated string
164164
/// @param sParam1 NWScript function argument. Null-terminated string
165165
/// @param nParam2 NWScript function argument.
166-
/// @param result Buffer for storing the returned string. **Must be null-terminated**.
167-
/// @param resultSize Size of the result buffer
168-
__declspec(dllexport) void __cdecl NWNXCPlugin_GetString(void* cplugin,
169-
const char* sFunction,
170-
const char* sParam1,
171-
int32_t nParam2,
172-
char* result,
173-
size_t resultSize);
166+
/// @param buffer Buffer allocated by nwn2 for storing the result.
167+
/// @param bufferSize Size of the buffer
168+
/// @return The resulting null-terminated string pointer. Can be `buffer` or an already allocated
169+
/// string.
170+
__declspec(dllexport) const char* __cdecl NWNXCPlugin_GetString(void* cplugin,
171+
const char* sFunction,
172+
const char* sParam1,
173+
int32_t nParam2,
174+
char* buffer,
175+
size_t bufferSize);
174176

175177
/// Executed by NWScript function `NWNXSetString`
176178
/// @param cplugin User data pointer, as returned by `NWNXCPlugin_New`

‎src/hook/CPlugin.cpp

+20-1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ struct InitInfo {
4949
};
5050

5151
typedef void*(__cdecl NewPluginFn)(InitInfo info);
52+
typedef void(__cdecl GetStringFn)(void* cplugin,
53+
const char* sFunction,
54+
const char* sParam1,
55+
int32_t nParam2,
56+
char* result,
57+
size_t resultSize);
5258

5359
} // namespace CompatV1
5460

@@ -66,6 +72,8 @@ CPlugin::CPlugin(HINSTANCE hDLL, const CPlugin::InitInfo& initInfo)
6672
= reinterpret_cast<CompatV1::NewPluginFn*>(GetProcAddress(hDLL, "NWNXCPlugin_New"));
6773
m_dll.newPlugin = [newPlugin](const InitInfo* info) {
6874
auto hooks = CompatV1::NWN2Hooks {
75+
// v2 added 1 additional parameter for ExecuteScript & ExecuteScriptEnhanced (see
76+
// commit edc229f)
6977
.ExecuteScript = CompatV1::ExecuteScript,
7078
.ExecuteScriptEnhanced = CompatV1::ExecuteScriptEnhanced,
7179
.AddScriptParameterInt = info->nwn2_hooks->AddScriptParameterInt,
@@ -74,6 +82,7 @@ CPlugin::CPlugin(HINSTANCE hDLL, const CPlugin::InitInfo& initInfo)
7482
.AddScriptParameterObject = info->nwn2_hooks->AddScriptParameterObject,
7583
.ClearScriptParams = info->nwn2_hooks->ClearScriptParams,
7684
};
85+
// v2 reordered InitInfo members
7786
auto initInfo = CompatV1::InitInfo {
7887
.dll_path = info->dll_path,
7988
.nwnx_user_path = info->nwnx_user_path,
@@ -83,9 +92,20 @@ CPlugin::CPlugin(HINSTANCE hDLL, const CPlugin::InitInfo& initInfo)
8392
.nwnx_install_path = info->nwnx_install_path,
8493
.nwn2_hooks = &hooks,
8594
};
95+
// v2 is passing initInfo as a const pointer
8696
return newPlugin(initInfo);
8797
};
8898

99+
// v2 changed NWNXCPlugin_GetString behaviour: now NWNXCPlugin_GetString must return the
100+
// result string pointer, to avoid unnecessary strcpy from a stored value into `result`
101+
auto getString = reinterpret_cast<CompatV1::GetStringFn*>(
102+
GetProcAddress(hDLL, "NWNXCPlugin_GetString"));
103+
m_dll.getString = [getString](void* cplugin, const char* sFunction, const char* sParam1,
104+
int32_t nParam2, char* result, size_t resultSize) {
105+
getString(cplugin, sFunction, sParam1, nParam2, result, resultSize);
106+
return result;
107+
};
108+
89109
// clang-format off
90110
m_dll.deletePlugin = reinterpret_cast<DeletePluginFn*>(GetProcAddress(hDLL, "NWNXCPlugin_Delete"));
91111
m_dll.getInfo = reinterpret_cast<GetInfoFn*>( GetProcAddress(hDLL, "NWNXCPlugin_GetInfo"));
@@ -95,7 +115,6 @@ CPlugin::CPlugin(HINSTANCE hDLL, const CPlugin::InitInfo& initInfo)
95115
m_dll.setInt = reinterpret_cast<SetIntFn*>( GetProcAddress(hDLL, "NWNXCPlugin_SetInt"));
96116
m_dll.getFloat = reinterpret_cast<GetFloatFn*>( GetProcAddress(hDLL, "NWNXCPlugin_GetFloat"));
97117
m_dll.setFloat = reinterpret_cast<SetFloatFn*>( GetProcAddress(hDLL, "NWNXCPlugin_SetFloat"));
98-
m_dll.getString = reinterpret_cast<GetStringFn*>( GetProcAddress(hDLL, "NWNXCPlugin_GetString"));
99118
m_dll.setString = reinterpret_cast<SetStringFn*>( GetProcAddress(hDLL, "NWNXCPlugin_SetString"));
100119
m_dll.getGFFSize = reinterpret_cast<GetGFFSizeFn*>( GetProcAddress(hDLL, "NWNXCPlugin_GetGFFSize"));
101120
m_dll.getGFF = reinterpret_cast<GetGFFFn*>( GetProcAddress(hDLL, "NWNXCPlugin_GetGFF"));

‎src/hook/CPlugin.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,13 @@ class CPlugin {
8181
m_dll.setFloat(m_instancePtr, sFunction, sParam1, nParam2, fValue);
8282
}
8383
}
84-
inline void GetString(
84+
inline const char* GetString(
8585
const char* sFunction, const char* sParam1, int nParam2, char* result, size_t resultSize)
8686
{
8787
if (m_dll.getString) {
8888
return m_dll.getString(m_instancePtr, sFunction, sParam1, nParam2, result, resultSize);
8989
}
90+
return result;
9091
}
9192
inline void SetString(const char* sFunction, const char* sParam1, int nParam2, char* sValue)
9293
{
@@ -127,7 +128,7 @@ class CPlugin {
127128
typedef void (__cdecl SetIntFn) (void* cplugin, const char* sFunction, const char* sParam1, int32_t nParam2, int32_t nValue);
128129
typedef float (__cdecl GetFloatFn) (void* cplugin, const char* sFunction, const char* sParam1, int32_t nParam2);
129130
typedef void (__cdecl SetFloatFn) (void* cplugin, const char* sFunction, const char* sParam1, int32_t nParam2, float fValue);
130-
typedef void (__cdecl GetStringFn) (void* cplugin, const char* sFunction, const char* sParam1, int32_t nParam2, char* result, size_t resultSize);
131+
typedef const char* (__cdecl GetStringFn) (void* cplugin, const char* sFunction, const char* sParam1, int32_t nParam2, char* result, size_t resultSize);
131132
typedef void (__cdecl SetStringFn) (void* cplugin, const char* sFunction, const char* sParam1, int32_t nParam2, const char* sValue);
132133
typedef size_t (__cdecl GetGFFSizeFn) (void* cplugin, const char* sVarName);
133134
typedef void (__cdecl GetGFFFn) (void* cplugin, const char* sVarName, uint8_t* result, size_t resultSize);

‎src/hook/hook.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,7 @@ char* NWNXGetString(char* sPlugin, char* sFunction, char* sParam1, int nParam2)
286286
auto cpluginIt = cplugins.find(sPlugin);
287287
if (cpluginIt != cplugins.end()) {
288288
returnBuffer[0] = '\0';
289-
cpluginIt->second->GetString(sFunction, sParam1, nParam2, returnBuffer, MAX_BUFFER);
290-
return returnBuffer;
289+
return cpluginIt->second->GetString(sFunction, sParam1, nParam2, returnBuffer, MAX_BUFFER);
291290
}
292291

293292
// Plugin class forwarding

‎src/plugins/xp_example_cplugin/cplugin_example.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,12 @@ void NWNXCPlugin_SetFloat(
185185
plugin->storedFloat = fValue;
186186
}
187187

188-
void NWNXCPlugin_GetString(void* cplugin,
189-
const char* sFunction,
190-
const char* sParam1,
191-
int32_t nParam2,
192-
char* result,
193-
size_t resultSize)
188+
const char* NWNXCPlugin_GetString(void* cplugin,
189+
const char* sFunction,
190+
const char* sParam1,
191+
int32_t nParam2,
192+
char* result,
193+
size_t resultSize)
194194
{
195195
auto plugin = static_cast<Plugin*>(cplugin);
196196

@@ -199,7 +199,7 @@ void NWNXCPlugin_GetString(void* cplugin,
199199

200200
// Copy stored string into result buffer
201201
plugin->logFile << " Return " << plugin->storedString << std::endl;
202-
strncpy_s(result, resultSize, plugin->storedString.c_str(), plugin->storedString.size());
202+
return plugin->storedString.c_str();
203203
}
204204

205205
void NWNXCPlugin_SetString(

0 commit comments

Comments
 (0)