compat/win/sys/types.h - Issue 883773005: win: Work towards getting 'minidump' to compile

Unified Diff: compat/win/sys/types.h

Issue 883773005: win: Work towards getting 'minidump' to compile (Closed) Base URL: https://chromium.googlesource.com/crashpad/crashpad@crash-report-db-win

Patch Set: . Created 5 years, 11 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: compat/win/sys/types.h

diff --git a/compat/win/sys/types.h b/compat/win/sys/types.h

index 9b46cc260343d68eec991b63e03bdbea290bd5d3..cfd2ccaa7ad48c6de2ac21550afa067c8c99b618 100644

--- a/compat/win/sys/types.h

+++ b/compat/win/sys/types.h

@@ -21,7 +21,9 @@

#ifdef _WIN64

typedef __int64 ssize_t;

#else

-typedef _W64 int ssize_t;

Mark Mentovai 2015/02/02 22:10:28 I thought _W64 sounded correct for this when you a

I thought _W64 sounded correct for this when you added it. It matched the declaration of size_t from the SDK. No good?

scottmg 2015/02/02 22:31:45 Yeah, it does, I was just trying to avoid the need

Show quoted text

On 2015/02/02 22:10:28, Mark Mentovai wrote: > I thought _W64 sounded correct for this when you added it. It matched the > declaration of size_t from the SDK. No good?

Yeah, it does, I was just trying to avoid the need to include another header here (windows.h) which could be confusing from sys/types.h. Instead, I should have switched to __w64, which I've done now. Ref: https://msdn.microsoft.com/en-us/library/s04b5w00.aspx

Mark Mentovai 2015/02/02 22:57:21 scottmg wrote:

scottmg wrote:

Show quoted text

> On 2015/02/02 22:10:28, Mark Mentovai wrote: > > I thought _W64 sounded correct for this when you added it. It matched the > > declaration of size_t from the SDK. No good? > > Yeah, it does, I was just trying to avoid the need to include another header > here (windows.h) which could be confusing from sys/types.h. > > Instead, I should have switched to __w64, which I've done now. Ref: > https://msdn.microsoft.com/en-us/library/s04b5w00.aspx

How does this work when _W64 didn’t? It looks like _W64 can only be __w64 or blank.

scottmg 2015/02/02 23:16:37 That's right. __w64 is compiler defined, not by an

Show quoted text

On 2015/02/02 22:57:21, Mark Mentovai wrote: > scottmg wrote: > > On 2015/02/02 22:10:28, Mark Mentovai wrote: > > > I thought _W64 sounded correct for this when you added it. It matched the > > > declaration of size_t from the SDK. No good? > > > > Yeah, it does, I was just trying to avoid the need to include another header > > here (windows.h) which could be confusing from sys/types.h. > > > > Instead, I should have switched to __w64, which I've done now. Ref: > > https://msdn.microsoft.com/en-us/library/s04b5w00.aspx > > How does this work when _W64 didn’t? It looks like _W64 can only be __w64 or > blank.

That's right. __w64 is compiler defined, not by any header. _W64 is the Windows SDK wrapper around that to avoid having to #if every time, but since we're already doing that to change the size of the type, _W64 will always be __w64 here.

Mark Mentovai 2015/02/02 23:32:08 Right, my point is: 1. _W64 was always __w64 for

Show quoted text

On 2015/02/02 23:16:37, scottmg wrote: > On 2015/02/02 22:57:21, Mark Mentovai wrote: > > scottmg wrote: > > > On 2015/02/02 22:10:28, Mark Mentovai wrote: > > > > I thought _W64 sounded correct for this when you added it. It matched the > > > > declaration of size_t from the SDK. No good? > > > > > > Yeah, it does, I was just trying to avoid the need to include another header > > > here (windows.h) which could be confusing from sys/types.h. > > > > > > Instead, I should have switched to __w64, which I've done now. Ref: > > > https://msdn.microsoft.com/en-us/library/s04b5w00.aspx > > > > How does this work when _W64 didn’t? It looks like _W64 can only be __w64 or > > blank. > > That's right. __w64 is compiler defined, not by any header. _W64 is the Windows > SDK wrapper around that to avoid having to #if every time, but since we're > already doing that to change the size of the type, _W64 will always be __w64 > here.

Right, my point is: 1. _W64 was always __w64 for us. 2. Something was broken and you removed _W64 to unbreak it. 3. Later, you switched this to __w64, and it remained unbroken. But given (1), how is (3) any different than what we had before?

scottmg 2015/02/03 18:12:56 Sorry, my explanations were too terse. The origin

Show quoted text

On 2015/02/02 23:32:08, Mark Mentovai wrote: > On 2015/02/02 23:16:37, scottmg wrote: > > On 2015/02/02 22:57:21, Mark Mentovai wrote: > > > scottmg wrote: > > > > On 2015/02/02 22:10:28, Mark Mentovai wrote: > > > > > I thought _W64 sounded correct for this when you added it. It matched > the > > > > > declaration of size_t from the SDK. No good? > > > > > > > > Yeah, it does, I was just trying to avoid the need to include another > header > > > > here (windows.h) which could be confusing from sys/types.h. > > > > > > > > Instead, I should have switched to __w64, which I've done now. Ref: > > > > https://msdn.microsoft.com/en-us/library/s04b5w00.aspx > > > > > > How does this work when _W64 didn’t? It looks like _W64 can only be __w64 or > > > blank. > > > > That's right. __w64 is compiler defined, not by any header. _W64 is the > Windows > > SDK wrapper around that to avoid having to #if every time, but since we're > > already doing that to change the size of the type, _W64 will always be __w64 > > here. > > Right, my point is: > > 1. _W64 was always __w64 for us. > > 2. Something was broken and you removed _W64 to unbreak it. > > 3. Later, you switched this to __w64, and it remained unbroken. > > But given (1), how is (3) any different than what we had before?

Sorry, my explanations were too terse. The original problem was that this header was incorrect -- it didn't #include <windows.h>, so _W64 was only available sometimes by accident. But as it happened, windows.h was always included before this file in all of our translation units before this CL. One fix would be to leave it as _W64 and include windows.h here. But that seems heavy, since we know the value of _W64 will always be __w64 in this case.

Mark Mentovai 2015/02/03 18:26:32 Got it! Thanks for the explanation.

Show quoted text

On 2015/02/03 18:12:56, scottmg wrote: > On 2015/02/02 23:32:08, Mark Mentovai wrote: > > On 2015/02/02 23:16:37, scottmg wrote: > > > On 2015/02/02 22:57:21, Mark Mentovai wrote: > > > > scottmg wrote: > > > > > On 2015/02/02 22:10:28, Mark Mentovai wrote: > > > > > > I thought _W64 sounded correct for this when you added it. It matched > > the > > > > > > declaration of size_t from the SDK. No good? > > > > > > > > > > Yeah, it does, I was just trying to avoid the need to include another > > header > > > > > here (windows.h) which could be confusing from sys/types.h. > > > > > > > > > > Instead, I should have switched to __w64, which I've done now. Ref: > > > > > https://msdn.microsoft.com/en-us/library/s04b5w00.aspx > > > > > > > > How does this work when _W64 didn’t? It looks like _W64 can only be __w64 > or > > > > blank. > > > > > > That's right. __w64 is compiler defined, not by any header. _W64 is the > > Windows > > > SDK wrapper around that to avoid having to #if every time, but since we're > > > already doing that to change the size of the type, _W64 will always be __w64 > > > here. > > > > Right, my point is: > > > > 1. _W64 was always __w64 for us. > > > > 2. Something was broken and you removed _W64 to unbreak it. > > > > 3. Later, you switched this to __w64, and it remained unbroken. > > > > But given (1), how is (3) any different than what we had before? > > Sorry, my explanations were too terse. > > The original problem was that this header was incorrect -- it didn't #include > <windows.h>, so _W64 was only available sometimes by accident. But as it > happened, windows.h was always included before this file in all of our > translation units before this CL. > > One fix would be to leave it as _W64 and include windows.h here. But that seems > heavy, since we know the value of _W64 will always be __w64 in this case.

Got it! Thanks for the explanation.

+typedef int ssize_t;

#endif

+typedef unsigned long pid_t;

#endif // CRASHPAD_COMPAT_WIN_SYS_TYPES_H_

« no previous file with comments | « no previous file | minidump/minidump_context.h » ('j') | minidump/minidump_context.h » ('J')