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

Issue 338590043: [plaso] Replace binplist with biplist #1480 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months ago by onager
Modified:
5 months ago
Reviewers:
Joachim Metz
CC:
kiddi, log2timeline-dev_googlegroups.com
Visibility:
Public.

Description

[plaso] Replace binplist with biplist #1480

Patch Set 1 #

Total comments: 8

Patch Set 2 : Changes after review #

Patch Set 3 : Changes after rebase #

Total comments: 10

Patch Set 4 : Changes after review #

Total comments: 6

Patch Set 5 : Changes after review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -127 lines) Patch
M appveyor.yml View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M config/dpkg/control View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M config/linux/gift_copr_install.sh View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M config/linux/gift_ppa_install.sh View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M config/macos/install.sh View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M config/macos/make_dist.sh View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M config/macos/uninstall.sh View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M config/travis/install.sh View 1 2 3 1 chunk +3 lines, -3 lines 0 comments Download
M dependencies.ini View 1 2 3 1 chunk +4 lines, -5 lines 0 comments Download
M plaso/cli/log2timeline_tool.py View 1 2 3 chunks +0 lines, -7 lines 0 comments Download
D plaso/cli/logging_filter.py View 1 chunk +0 lines, -27 lines 0 comments Download
M plaso/dependencies.py View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M plaso/lib/plist.py View 1 2 3 4 3 chunks +14 lines, -18 lines 0 comments Download
M plaso/parsers/plist.py View 1 2 3 5 chunks +8 lines, -38 lines 0 comments Download
M plaso/parsers/plist_plugins/macuser.py View 1 2 3 4 3 chunks +6 lines, -7 lines 0 comments Download
M requirements.txt View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M setup.cfg View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M tests/parsers/plist.py View 1 2 3 1 chunk +4 lines, -4 lines 0 comments Download
M tests/parsers/plist_plugins/bluetooth.py View 1 2 2 chunks +5 lines, -5 lines 0 comments Download
M tests/parsers/plist_plugins/spotlight.py View 1 2 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 15
onager
7 months ago (2018-03-18 20:43:02 UTC) #1
Joachim Metz
overall looks good, couple of smaller things, I think there are 3 changes in here ...
6 months, 3 weeks ago (2018-03-22 06:06:39 UTC) #2
Joachim Metz
https://codereview.appspot.com/338590043/diff/1/.travis.yml File .travis.yml (right): https://codereview.appspot.com/338590043/diff/1/.travis.yml#newcode1 .travis.yml:1: language: python Per chat I opt to stage these ...
6 months, 3 weeks ago (2018-03-22 06:06:46 UTC) #3
onager
Code updated.
6 months, 3 weeks ago (2018-03-24 21:37:50 UTC) #4
onager
https://codereview.appspot.com/338590043/diff/1/.travis.yml File .travis.yml (right): https://codereview.appspot.com/338590043/diff/1/.travis.yml#newcode1 .travis.yml:1: language: python On 2018/03/22 06:06:45, Joachim Metz wrote: > ...
6 months, 3 weeks ago (2018-03-25 08:32:31 UTC) #5
Joachim Metz
now let's add biplist to the l2tdevtools plaso preset, so we can build a version ...
6 months, 2 weeks ago (2018-03-29 11:12:24 UTC) #6
Joachim Metz
Please update this CL biplist should now be l2tbinaries and GIFT PPA
6 months, 2 weeks ago (2018-03-31 11:42:57 UTC) #7
onager
Code updated.
5 months, 2 weeks ago (2018-05-04 11:28:50 UTC) #8
Joachim Metz
Please add the truncated test binary plist file I've mailed you https://codereview.appspot.com/338590043/diff/40001/.travis.yml File .travis.yml (left): ...
5 months, 1 week ago (2018-05-05 07:20:02 UTC) #9
onager
Code updated.
5 months ago (2018-05-16 09:33:21 UTC) #10
onager
https://codereview.appspot.com/338590043/diff/40001/.travis.yml File .travis.yml (left): https://codereview.appspot.com/338590043/diff/40001/.travis.yml#oldcode42 .travis.yml:42: - env: [TARGET="linux-python34-tox", TOXENV="py34"] On 2018/05/05 07:20:02, Joachim Metz ...
5 months ago (2018-05-16 09:34:08 UTC) #11
Joachim Metz
LGTM, with some remaining requests, questions, remarks https://codereview.appspot.com/338590043/diff/60001/plaso/lib/plist.py File plaso/lib/plist.py (right): https://codereview.appspot.com/338590043/diff/60001/plaso/lib/plist.py#newcode36 plaso/lib/plist.py:36: if isinstance(key, ...
5 months ago (2018-05-18 05:09:52 UTC) #12
onager
Code updated.
5 months ago (2018-05-18 11:29:52 UTC) #13
onager
Changes have been merged with master branch. To close the review and clean up the ...
5 months ago (2018-05-18 12:03:52 UTC) #14
onager
5 months ago (2018-05-18 12:29:57 UTC) #15
https://codereview.appspot.com/338590043/diff/60001/plaso/lib/plist.py
File plaso/lib/plist.py (right):

https://codereview.appspot.com/338590043/diff/60001/plaso/lib/plist.py#newcode36
plaso/lib/plist.py:36: if isinstance(key, (dict, plistlib._InternalDict)):
On 2018/05/18 05:09:52, Joachim Metz wrote:
> Does biplist generate objects of type plistlib._InternalDict? Otherwise we
could
> likely remove this code?

Done.

https://codereview.appspot.com/338590043/diff/60001/plaso/lib/plist.py#newcode59
plaso/lib/plist.py:59: file_object: the file-like object.
On 2018/05/18 05:09:52, Joachim Metz wrote:
> This appears to be an old docstring style, could you please update as well,
> seeing you're touching this file. Thx

Done.

https://codereview.appspot.com/338590043/diff/60001/plaso/parsers/plist_plugi...
File plaso/parsers/plist_plugins/macuser.py (right):

https://codereview.appspot.com/338590043/diff/60001/plaso/parsers/plist_plugi...
plaso/parsers/plist_plugins/macuser.py:137: plist_file = dict()
On 2018/05/18 05:09:52, Joachim Metz wrote:
> should we generate a processing error instead of falling back to empty dict?
> 
> I also opt to use {} instead of dict() which is supposedly slightly faster:
>
https://stackoverflow.com/questions/5790860/and-vs-list-and-dict-which-is-better

We could, many of the MacOS parsers are quite old, and could do with general
clean and refactoring. Filed https://github.com/log2timeline/plaso/issues/1888
for that.
Sign in to reply to this message.

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