-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang][runtime] Clean up code to unblock development #78063
Conversation
@llvm/pr-subscribers-flang-runtime Author: Peter Klausler (klausler) ChangesClean up recently-added code to avoid warnings and to eliminate a needless dependence from the Fortran runtime support library on C++ runtimes. Full diff: https://github.com/llvm/llvm-project/pull/78063.diff 4 Files Affected:
diff --git a/flang/include/flang/Runtime/extensions.h b/flang/include/flang/Runtime/extensions.h
index b34edb94ada43a..1ed750f3b70e0f 100644
--- a/flang/include/flang/Runtime/extensions.h
+++ b/flang/include/flang/Runtime/extensions.h
@@ -30,10 +30,10 @@ std::int32_t FORTRAN_PROCEDURE_NAME(iargc)();
// GNU Fortran 77 compatibility subroutine GETARG(N, ARG).
void FORTRAN_PROCEDURE_NAME(getarg)(
- std::int32_t &n, std::int8_t *arg, std::int64_t length);
+ std::int32_t &n, char *arg, std::int64_t length);
// GNU extension subroutine GETLOG(C).
-void FORTRAN_PROCEDURE_NAME(getlog)(std::byte *name, std::int64_t length);
+void FORTRAN_PROCEDURE_NAME(getlog)(char *name, std::int64_t length);
} // extern "C"
#endif // FORTRAN_RUNTIME_EXTENSIONS_H_
diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index c327f07f5cbffe..ff8fef5d7cb758 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -151,19 +151,19 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
// add "cmd.exe /c " to the beginning of command
const char *prefix{"cmd.exe /c "};
- char *newCmdWin{(char *)AllocateMemoryOrCrash(
+ char *newCmdWin{AllocateMemoryOrCrash(
terminator, std::strlen(prefix) + std::strlen(newCmd) + 1)};
std::strcpy(newCmdWin, prefix);
std::strcat(newCmdWin, newCmd);
// Convert the char to wide char
const size_t sizeNeeded{mbstowcs(NULL, newCmdWin, 0) + 1};
- wchar_t *wcmd{(wchar_t *)AllocateMemoryOrCrash(
- terminator, sizeNeeded * sizeof(wchar_t))};
+ wchar_t *wcmd{
+ AllocateMemoryOrCrash(terminator, sizeNeeded * sizeof(wchar_t))};
if (std::mbstowcs(wcmd, newCmdWin, sizeNeeded) == static_cast<size_t>(-1)) {
terminator.Crash("Char to wide char failed for newCmd");
}
- FreeMemory((void *)newCmdWin);
+ FreeMemory(newCmdWin);
if (CreateProcess(nullptr, wcmd, nullptr, nullptr, FALSE, 0, nullptr,
nullptr, &si, &pi)) {
@@ -179,7 +179,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
CheckAndCopyCharsToDescriptor(cmdmsg, "CreateProcess failed.");
}
}
- FreeMemory((void *)wcmd);
+ FreeMemory(wcmd);
#else
// terminated children do not become zombies
signal(SIGCHLD, SIG_IGN);
@@ -200,7 +200,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
}
// Deallocate memory if EnsureNullTerminated dynamically allocated memory
if (newCmd != command.OffsetElement()) {
- FreeMemory((void *)newCmd);
+ FreeMemory(newCmd);
}
}
diff --git a/flang/runtime/extensions.cpp b/flang/runtime/extensions.cpp
index 352da8f11d9d56..38a0633a3648d7 100644
--- a/flang/runtime/extensions.cpp
+++ b/flang/runtime/extensions.cpp
@@ -47,8 +47,7 @@ extern "C" {
namespace Fortran::runtime {
-void GetUsernameEnvVar(
- const char *envName, std::byte *arg, std::int64_t length) {
+void GetUsernameEnvVar(const char *envName, char *arg, std::int64_t length) {
Descriptor name{*Descriptor::Create(
1, std::strlen(envName) + 1, const_cast<char *>(envName), 0)};
Descriptor value{*Descriptor::Create(1, length, arg, 0)};
@@ -91,36 +90,23 @@ std::int32_t FORTRAN_PROCEDURE_NAME(iargc)() { return RTNAME(ArgumentCount)(); }
// CALL GETARG(N, ARG)
void FORTRAN_PROCEDURE_NAME(getarg)(
- std::int32_t &n, std::int8_t *arg, std::int64_t length) {
+ std::int32_t &n, char *arg, std::int64_t length) {
Descriptor value{*Descriptor::Create(1, length, arg, 0)};
(void)RTNAME(GetCommandArgument)(
n, &value, nullptr, nullptr, __FILE__, __LINE__);
}
// CALL GETLOG(USRNAME)
-void FORTRAN_PROCEDURE_NAME(getlog)(std::byte *arg, std::int64_t length) {
+void FORTRAN_PROCEDURE_NAME(getlog)(char *arg, std::int64_t length) {
#if _REENTRANT || _POSIX_C_SOURCE >= 199506L
- int nameMaxLen;
-#ifdef LOGIN_NAME_MAX
- nameMaxLen = LOGIN_NAME_MAX + 1;
-#else
- nameMaxLen = sysconf(_SC_LOGIN_NAME_MAX) + 1;
- if (nameMaxLen == -1)
- nameMaxLen = _POSIX_LOGIN_NAME_MAX + 1;
-#endif
- std::vector<char> str(nameMaxLen);
-
- int error{getlogin_r(str.data(), nameMaxLen)};
- if (error == 0) {
- // no error: find first \0 in string then pad from there
- CopyAndPad(reinterpret_cast<char *>(arg), str.data(), length,
- std::strlen(str.data()));
- } else {
- // error occur: get username from environment variable
- GetUsernameEnvVar("LOGNAME", arg, length);
+ if (length >= 1 && getlogin_r(arg, length) == 0) {
+ auto loginLen{std::strlen(arg)};
+ std::memset(
+ arg + loginLen, ' ', static_cast<std::size_t>(length) - loginLen);
+ return;
}
-#elif _WIN32
- // Get username from environment to avoid link to Advapi32.lib
+#endif
+#if _WIN32
GetUsernameEnvVar("USERNAME", arg, length);
#else
GetUsernameEnvVar("LOGNAME", arg, length);
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index a0f14c519412a4..3c9432a47e8b58 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -681,10 +681,7 @@ TEST_F(EnvironmentVariables, ErrMsgTooShort) {
TEST_F(EnvironmentVariables, GetlogGetName) {
const int charLen{3};
char input[charLen]{"\0\0"};
-
- FORTRAN_PROCEDURE_NAME(getlog)
- (reinterpret_cast<std::byte *>(input), charLen);
-
+ FORTRAN_PROCEDURE_NAME(getlog)(input, charLen);
EXPECT_NE(input[0], '\0');
}
@@ -700,10 +697,7 @@ TEST_F(EnvironmentVariables, GetlogPadSpace) {
charLen = _POSIX_LOGIN_NAME_MAX + 2;
#endif
std::vector<char> input(charLen);
-
- FORTRAN_PROCEDURE_NAME(getlog)
- (reinterpret_cast<std::byte *>(input.data()), charLen);
-
+ FORTRAN_PROCEDURE_NAME(getlog)(input.data(), charLen);
EXPECT_EQ(input[charLen - 1], ' ');
}
#endif
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the cleanup
108ff0b
to
f4ca8f7
Compare
Clean up recently-added code to avoid warnings and to eliminate a needless dependence from the Fortran runtime support library on C++ runtimes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't been able to build or test this, as I normally would. But it looks good to me.
Thanks for doing this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for rewriting this, the check flow for getlog looks better now
Clean up recently-added code to avoid warnings and to eliminate a needless dependence from the Fortran runtime support library on C++ runtimes.
Clean up recently-added code to avoid warnings and to eliminate a needless dependence from the Fortran runtime support library on C++ runtimes.