Skip to content
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

Adds Process struct for use with the ScriptCompilationSystem in RED4ext #79

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jackhumbert
Copy link
Contributor

Paired with WopsS/RED4ext#23

Copy link
Owner

@WopsS WopsS left a comment

Choose a reason for hiding this comment

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

Thanks! Again some remarks.

include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
include/RED4ext/RedProcess.hpp Outdated Show resolved Hide resolved
@jackhumbert jackhumbert changed the title Adds red::Process for use with the ScriptCompilationSystem in RED4ext Adds Process struct for use with the ScriptCompilationSystem in RED4ext May 11, 2023
Comment on lines 13 to 18
struct FixedWString
{
uint32_t length;
uint32_t maxLength;
const wchar_t* str;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't it be WString or WideString or CWideString? Capacity field suggests that this is a mutable string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right - it looks like this is used elsewhere too. I'll do some digging and see what I can find.

Copy link
Owner

@WopsS WopsS left a comment

Choose a reason for hiding this comment

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

Few more remarks 🏃

Comment on lines +8 to +17
// This file has been generated by zoltan (https://github.com/jac3km4/zoltan)

#define Process_CloseHandles_Addr 0x2C23B30
#define Process_Execute_Addr 0x2C23D70
#define Process_Execute_0_Addr 0x2C23DD0
#define Process_GetExitCode_Addr 0x2C23B60
#define Process_Process_Addr 0x2C23AA0
#define Process_ReadFromPipe_Addr 0x2C23B90
#define Process_Terminate_Addr 0x2C24040
#define Process_WaitUntilCompleted_Addr 0x2C240B0
Copy link
Owner

Choose a reason for hiding this comment

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

Please move these to IDA script.

#define Process_Terminate_Addr 0x2C24040
#define Process_WaitUntilCompleted_Addr 0x2C240B0

inline bool RED4ext::Process::CloseHandles()
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
inline bool RED4ext::Process::CloseHandles()
RED4EXT_INLINE bool RED4ext::Process::CloseHandles()

Same for the others.

*
* @return Returns true if the path was added is attached, false otherwise.
*
* @pattern 48 89 5C 24 08 57 48 83 EC 40 33 C0 C7 44 24 20 18 00 00 00 48 8D 91 08 20 00 00 48 89 81 00 20
Copy link
Owner

Choose a reason for hiding this comment

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

Please move these pattern in the IDA script.

return call(this, a1, a2, a3, a4);
}

inline uint64_t RED4ext::Process::GetExitCode()
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
inline uint64_t RED4ext::Process::GetExitCode()
RED4EXT_INLINE DWORD RED4ext::Process::GetExitCode()

Comment on lines +20 to +38
enum class ExecutionFlags : std::uint8_t
{
// unsets CREATE_NO_WINDOW
ShouldCreateWindow = 0x1,
// sets CREATE_BREAKAWAY_FROM_JOB | CREATE_SUSPENDED in Process Creation Flags
BreakawayAndSuspend = 0x2,
// unsets bInheritHandles
NoInheritHandles = 0x4
};

enum class ReadFlags : std::uint8_t
{
// open & read regardless of other options
Unk1 = 0x1,
// opens file from name provided and writes output
WriteToFile = 0x2,
// writes output to CString
GetCString = 0x4
};
Copy link
Owner

Choose a reason for hiding this comment

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

I remembered that we decided to use struct flags, e.g. https://github.com/WopsS/RED4ext.SDK/blob/master/include/RED4ext/RTTITypes.hpp#L117-L132, so please convert these a as well to be consistent.

Comment on lines +112 to +113
// DEFINE_ENUM_FLAG_OPERATORS(Process::ExecutionFlags);
// DEFINE_ENUM_FLAG_OPERATORS(Process::ReadFlags);
Copy link
Owner

Choose a reason for hiding this comment

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

If commented, just remove it.

Comment on lines +5 to +6
#endif
#include <RED4ext/Relocation.hpp>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
#endif
#include <RED4ext/Relocation.hpp>
#endif
#include <RED4ext/Relocation.hpp>

@@ -0,0 +1,73 @@
#pragma once
Copy link
Owner

Choose a reason for hiding this comment

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

Please add the cpp file too.

@jackhumbert jackhumbert marked this pull request as draft May 11, 2023 17:02
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