Fixes #31 rSharp_create_domain need an argument for library location … #65
Conversation
|
What about
This path also must be constructured from the provided path. |
src/rSharp.cpp
Outdated
| delete[] wideStringPath; | ||
| } | ||
|
|
||
| char_t* MergeLibPath(char** libPath, char* additionalPath) |
There was a problem hiding this comment.
Make both arguments into wchar_t instead of char. This method will only concatenate the stuff, not widen the characters and concatenate.
src/rSharp.cpp
Outdated
| void rSharp_create_domain(char ** libPath) | ||
| { | ||
| char_t* wideStringPath = MergeLibPath(libPath, "/RSharp.runtimeconfig.json"); | ||
| dotnetlib_path = MergeLibPath(libPath, "/ClrFacade.dll"); |
There was a problem hiding this comment.
both constant strings on this line and the line above could be STR("the string in here") because then you don't have to allocate memory for the widening conversion. They will be compiled as wchar_t
There was a problem hiding this comment.
Also, move this below the blocking test for nullptr. We don't need to do any string concatenation if the assembly pointer is already initialized.
src/rSharp.h
Outdated
| @@ -144,6 +144,7 @@ extern "C" { | |||
| RSharpGenericValue ConvertToRSharpGenericValue(SEXP s); | |||
| RSharpGenericValue** sexp_to_parameters(SEXP args); | |||
| SEXP ConvertToSEXP(RSharpGenericValue& value); | |||
| char_t* MergeLibPath(char** libPath, char* additionalPath); | |||
There was a problem hiding this comment.
There is lots of cleanup required in this file. In general, we should not expose the prototypes for functions that are not 'public'.
Put this prototype in the rSharp.c file toward the top.
src/rSharp.cpp
Outdated
|
|
||
| char_t* MergeLibPath(char** libPath, char* additionalPath) | ||
| { | ||
| size_t libPathLen = strlen(*libPath); |
There was a problem hiding this comment.
This is another one of those situations where there's lots of cleanup to do, but let's start eliminating abbreviations from variable and function/method names.
There was a problem hiding this comment.
It's much more clear for me to read libararyPathLength or library_path_length
src/rSharp.cpp
Outdated
| mbstowcs_s(nullptr, wideStringPath, length + 1, combinedPath, length); | ||
|
|
||
| delete[] combinedPath; | ||
| return wideStringPath; |
PavelBal
deleted the
31-rsharp_create_domain-need-an-argument-for-library-location
branch
Fixes #31 rSharp_create_domain need an argument for library location
Fixes #30 Statically link nethost in Windows
Update code to allow the path for the library to be passed from R to the C++ code.