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

Issue 276001: Use task_info(..., TASK_DYLD_INFO, ...) on 10.6 and later (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years ago by Mark Mentovai
Modified:
12 years, 8 months ago
Reviewers:
mochalatte
CC:
google-breakpad-dev_googlegroups.com
Base URL:
http://google-breakpad.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Use task_info(..., TASK_DYLD_INFO, ...) on 10.6 and later in preference to
looking up the _dyld_all_image_infos symbol in /usr/lib/dyld.

Landed

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/client/mac/Breakpad.xcodeproj/project.pbxproj View 11 chunks +14 lines, -3 lines 0 comments Download
M src/client/mac/handler/dynamic_images.cc View 1 2 chunks +57 lines, -5 lines 2 comments Download

Messages

Total messages: 3
Mark Mentovai
13 years ago #1
mochalatte
LGTM http://breakpad.appspot.com/276001/diff/2001/4 File src/client/mac/handler/dynamic_images.cc (right): http://breakpad.appspot.com/276001/diff/2001/4#newcode375 Line 375: #else nit: i don't think this preprocessor ...
13 years ago #2
Mark Mentovai
13 years ago #3
http://breakpad.appspot.com/276001/diff/2001/4
File src/client/mac/handler/dynamic_images.cc (right):

http://breakpad.appspot.com/276001/diff/2001/4#newcode375
Line 375: #else
mochalatte wrote:
> nit: i don't think this preprocessor optimization is necessary, it seem
simpler
> to just use the OS API all the time, especially since it's cached via the
static
> function variable in GetOSVersion

I disagree. This will cause all of the static functions to disappear, and
GetDyldAllImageInfosPointer to have the impossible branch optimized out. It’s a
code size win for any products that don’t care about 10.5 or earlier. This is a
fairly standard way that I handle OS feature availability.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted