Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(767)

Issue 1650013002: Revert of [Courgette] Refactor: Manage AssemblyProgram and EncodedProgram with scoped_ptr. (Closed)

Created:
4 years, 10 months ago by dcheng
Modified:
4 years, 10 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [Courgette] Refactor: Manage AssemblyProgram and EncodedProgram with scoped_ptr. (patchset #5 id:80001 of https://codereview.chromium.org/1629703002/ ) Reason for revert: Breaks ninja build on Linux GN. Original issue's description: > [Courgette] Refactor: Manage AssemblyProgram and EncodedProgram with scoped_ptr. > > Previously naked pointers AssemblyProgram and EncodedProgram are used over the > place, and are deallocated using Delete{AssemblyProgram, EncodedProgram}(). > In this CL we use scoped_ptr to manage the life cycles of these objects. > > - Removed DeleteAssemblyProgram() and DeleteEncodedProgram() and replaced calls > with e.g., program.reset(nullptr); if the manual deallocation is a peak > memory optimization. > - Moved Encode() and ReadEncodedProgram() to the .h files matching the .cc files. > - Extracted DetectExecutableType() and ParseDetectedExecutable() from > disassembly.* to new files program_detector*c, since Disassembly is really an > implementation that caller's don't care about. > > Committed: https://crrev.com/0a9cbf1781a114b35a4e0f4a834f2d24ade2e917 > Cr-Commit-Position: refs/heads/master@{#372212} TBR=wfh@chromium.org,grt@chromium.org,huangs@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Committed: https://crrev.com/7958406fb94ffb798859006fef04dbd0d4164079 Cr-Commit-Position: refs/heads/master@{#372274}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+303 lines, -324 lines) Patch
M courgette/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M courgette/adjustment_method_unittest.cc View 4 chunks +23 lines, -30 lines 0 comments Download
M courgette/assembly_program.h View 3 chunks +2 lines, -9 lines 0 comments Download
M courgette/assembly_program.cc View 7 chunks +28 lines, -22 lines 0 comments Download
M courgette/courgette.h View 2 chunks +35 lines, -1 line 0 comments Download
M courgette/courgette.gyp View 1 chunk +0 lines, -2 lines 0 comments Download
M courgette/courgette_tool.cc View 9 chunks +37 lines, -44 lines 0 comments Download
M courgette/disassembler.cc View 1 chunk +93 lines, -0 lines 0 comments Download
M courgette/encode_decode_unittest.cc View 3 chunks +11 lines, -16 lines 0 comments Download
M courgette/encoded_program.h View 2 chunks +0 lines, -7 lines 0 comments Download
M courgette/encoded_program.cc View 4 chunks +14 lines, -10 lines 0 comments Download
M courgette/encoded_program_fuzz_unittest.cc View 5 chunks +13 lines, -15 lines 0 comments Download
M courgette/encoded_program_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M courgette/ensemble.cc View 2 chunks +3 lines, -1 line 0 comments Download
M courgette/patch_generator_x86_32.h View 4 chunks +29 lines, -25 lines 0 comments Download
M courgette/patcher_x86_32.h View 3 chunks +14 lines, -12 lines 0 comments Download
D courgette/program_detector.h View 1 chunk +0 lines, -43 lines 0 comments Download
D courgette/program_detector.cc View 1 chunk +0 lines, -85 lines 0 comments Download

Messages

Total messages: 8 (1 generated)
dcheng
Created Revert of [Courgette] Refactor: Manage AssemblyProgram and EncodedProgram with scoped_ptr.
4 years, 10 months ago (2016-01-29 03:13:03 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1650013002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1650013002/1
4 years, 10 months ago (2016-01-29 03:13:27 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-01-29 03:14:34 UTC) #3
Will Harris
On 2016/01/29 03:14:34, commit-bot: I haz the power wrote: > Committed patchset #1 (id:1) lgtm ...
4 years, 10 months ago (2016-01-29 03:15:14 UTC) #4
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/7958406fb94ffb798859006fef04dbd0d4164079 Cr-Commit-Position: refs/heads/master@{#372274}
4 years, 10 months ago (2016-01-29 03:15:45 UTC) #6
huangs
Sorry for the trouble, and thanks for taking care of this!
4 years, 10 months ago (2016-01-29 15:02:14 UTC) #7
huangs
4 years, 10 months ago (2016-01-29 15:44:23 UTC) #8
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1648313002/ by huangs@chromium.org.

The reason for reverting is: Relanding patch set with fix..

Powered by Google App Engine
This is Rietveld 408576698