Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1365)

Issue 8690: Fix build on Linux IA-64

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 10 months ago by Hobbes
Modified:
12 years, 2 months ago
Reviewers:
chandlerc, Zhanyong
CC:
googletestframework_googlegroups.com
Base URL:
http://googletest.googlecode.com/svn/trunk/
Visibility:
Public.

Description

The clone call used by death tests is not available on IA-64 architectures. see http://linux.die.net/man/2/clone This patch merely disables death tests for IA-64.

Patch Set 1 #

Patch Set 2 : reordered checks #

Patch Set 3 : and now fixing the newlines #

Patch Set 4 : fixing comments, reordering GTEST_HAS_ macros, only defining GTEST_HAS_CLONE on Linux/non-IA64 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -10 lines) Patch
include/gtest/internal/gtest-port.h View 1 2 3 3 chunks +27 lines, -10 lines 3 comments Download

Messages

Total messages: 9
Hobbes
12 years, 10 months ago (2008-12-01 15:04:41 UTC) #1
chandlerc
Tiny nit, but otherwise this looks good. http://codereview.appspot.com/8690/diff/1/2 File include/gtest/internal/gtest-port.h (right): http://codereview.appspot.com/8690/diff/1/2#newcode332 Line 332: #if ...
12 years, 10 months ago (2008-12-02 18:09:59 UTC) #2
Hobbes
reordered checks
12 years, 10 months ago (2008-12-02 18:16:05 UTC) #3
Hobbes
and now fixing the newlines
12 years, 10 months ago (2008-12-02 18:18:12 UTC) #4
Zhanyong
http://codereview.appspot.com/8690/diff/9/10 File include/gtest/internal/gtest-port.h (right): http://codereview.appspot.com/8690/diff/9/10#newcode319 Line 319: // On IA-64 for example it isn't: see ...
12 years, 10 months ago (2008-12-02 18:27:07 UTC) #5
Hobbes
fixing comments, reordering GTEST_HAS_ macros, only defining GTEST_HAS_CLONE on Linux/non-IA64
12 years, 10 months ago (2008-12-02 19:09:07 UTC) #6
chandlerc
LGTM Much nicer, thanks! http://codereview.appspot.com/8690/diff/14/15 File include/gtest/internal/gtest-port.h (right): http://codereview.appspot.com/8690/diff/14/15#newcode43 Line 43: // GTEST_HAS_CLONE - Define ...
12 years, 10 months ago (2008-12-02 19:16:29 UTC) #7
Zhanyong
Looks good. I'll do some testing and then commit it to trunk. Cheers! http://codereview.appspot.com/8690/diff/14/15 File ...
12 years, 10 months ago (2008-12-02 20:01:45 UTC) #8
Hobbes
12 years, 10 months ago (2008-12-02 20:09:44 UTC) #9
http://codereview.appspot.com/8690/diff/14/15
File include/gtest/internal/gtest-port.h (right):

http://codereview.appspot.com/8690/diff/14/15#newcode43
Line 43: //   GTEST_HAS_CLONE          - Define it to 1/0 to indicate clone(2)
On 2008/12/02 20:01:46, Zhanyong wrote:
> On 2008/12/02 19:16:29, chandlerc wrote:
> > indicate _that_ clone(2) ...
> 
> I can fix this when I commit the patch.

Thanks. You might wanna fix GTEST_HAS_TR1_TUPLE as well.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b