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
[WIP] The ultimate ros amd64 bringup #361
Changes from 42 commits
91c8699
ff1eaad
15a08e8
cce3d16
286b2fb
12002d4
546c05b
1bc15af
1f6d61b
9e097de
aedb9f6
d139743
25064d0
75e11df
13ef07a
03920b1
235566d
1e326fb
c4fbc15
29db059
c24203c
c9d1f6a
e503f53
9335f8f
4b4a334
945d807
a26ae1d
bd078ab
7749bdc
177b3e7
52721c2
fbec870
d5bdd7e
4351118
5b1588c
1dc22b6
8366b2c
29d20df
10e086b
3252ac3
66eb02c
c93d924
c3380ef
c249c15
8b48097
e7e7e40
a9fc91d
6e0a3be
f062e2d
21177bb
ebe9bc4
c8bd63f
4c80d83
96f3020
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ BOOL QueryConfig(LPCTSTR ServiceName) | |
DWORD cbBytesNeeded = 0; | ||
LPQUERY_SERVICE_CONFIG pServiceConfig = NULL; | ||
LPWSTR lpPtr; | ||
INT nLen, i; | ||
SSIZE_T nLen, i; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need to keep these signed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not. I just did it this way to avoid signed vs unsigned issues. Remember this is all about making 64 bits great again, not about fixing unrelated stuff. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not, but I don't intend to change it., since it is unrelated and can potentially cause issues. |
||
|
||
#ifdef SCDBG | ||
_tprintf(_T("service to show configuration - %s\n\n"), ServiceName); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -169,7 +169,7 @@ ParseFailureActions( | |
SC_ACTION *pActions = NULL; | ||
LPTSTR pStringBuffer = NULL; | ||
LPTSTR p; | ||
INT nLength; | ||
INT_PTR nLength; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not SIZE_T ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same thing as with SSIZE_T. |
||
INT nCount = 0; | ||
|
||
*pcActions = 0; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -451,7 +451,8 @@ pretty_print_option(unsigned int code, unsigned char *data, int len, | |
static char optbuf[32768]; /* XXX */ | ||
int hunksize = 0, numhunk = -1, numelem = 0; | ||
char fmtbuf[32], *op = optbuf; | ||
int i, j, k, opleft = sizeof(optbuf); | ||
int i, j, k; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something's off about the indentation here. |
||
size_t opleft = sizeof(optbuf); | ||
unsigned char *dp = data; | ||
struct in_addr foo; | ||
char comma; | ||
|
@@ -552,7 +553,7 @@ pretty_print_option(unsigned int code, unsigned char *data, int len, | |
/* Cycle through the array (or hunk) printing the data. */ | ||
for (i = 0; i < numhunk; i++) { | ||
for (j = 0; j < numelem; j++) { | ||
int opcount; | ||
size_t opcount; | ||
switch (fmtbuf[j]) { | ||
case 't': | ||
if (emit_quotes) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,20 +70,20 @@ LogToFile(LPCWSTR lpMsg, | |
UINT flags) | ||
{ | ||
LPWSTR lpFullMsg = NULL; | ||
DWORD msgLen; | ||
SIZE_T msgLen; | ||
|
||
msgLen = wcslen(lpMsg) + 1; | ||
|
||
if (flags & LOG_ERROR) | ||
{ | ||
LPVOID lpSysMsg; | ||
LPWSTR lpSysMsg; | ||
DWORD eMsgLen; | ||
|
||
eMsgLen = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, | ||
NULL, | ||
errNum, | ||
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), | ||
(LPTSTR)&lpSysMsg, | ||
(LPWSTR)&lpSysMsg, | ||
0, | ||
NULL); | ||
|
||
|
@@ -130,7 +130,7 @@ LogToFile(LPCWSTR lpMsg, | |
|
||
bRet = WriteFile(hLogFile, | ||
lpFullMsg, | ||
wcslen(lpFullMsg) * sizeof(WCHAR), | ||
(DWORD)wcslen(lpFullMsg) * sizeof(WCHAR), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Many of these casts are questionable. This one is an example of one that I consider outright wrong. You're silencing a legitimate warning about integer overflow without fixing the problem. I'd rather have the warning to help us in the future when we actually go and overflow-proof the codebase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would require:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that these things are very unlikely to happen as accidents. Malicious actors tend to find a way to make them happen though, so it's a security concern. And I know that we only marginally care about security at this point -- which is why my recommendation was to just leave the warning be a warning. |
||
&bytesWritten, | ||
&olWrite); | ||
if (!bRet) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -837,8 +837,8 @@ SmpCreateVolumeDescriptors(VOID) | |
/* Query the device map so we can get the drive letters */ | ||
Status = NtQueryInformationProcess(NtCurrentProcess(), | ||
ProcessDeviceMap, | ||
&ProcessInformation, | ||
sizeof(ProcessInformation), | ||
&ProcessInformation.Query, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change looks fine to me though I'm curious what problem the other version caused. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the commit message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. Didn't see it stand out among the other 53 commits ;) |
||
sizeof(ProcessInformation.Query), | ||
NULL); | ||
if (!NT_SUCCESS(Status)) | ||
{ | ||
|
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.
This is code synced with Aleksander Telyatnikov's code (from uniata), so maybe it would be interesting to surround the changes with
#ifdef __REACTOS__ ... #endif
.