Issue 176693003: chromeos: Make dbus::MessageReader memory ownership explicit

Issue 176693003: chromeos: Make dbus::MessageReader memory ownership explicit (Closed)

Created:
6 years, 10 months ago by Daniel Erat

Modified:
6 years, 10 months ago

Reviewers:
satorux1, Ilya Sherman, Michael van Ouwerkerk

CC:
chromium-reviews, stevenjb+watch_chromium.org, oshima+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

chromeos: Make dbus::MessageReader memory ownership explicit Make memory returned by MessageReader::PopArrayOfBytes() const to make it clearer that ownership remains with the MessageReader. Also update PopArrayOfStrings() and PopArrayOfObjectPaths() to clear the passed-in vectors before appending to them. BUG=none TBR=isherman@chromium.org,mvanouwerkerk@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252922

Patch Set 1 #

Patch Set 2 : update a few more callers #

Total comments: 2

Patch Set 3 : remove unnecessary reinterpret_cast #

Created: 6 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+36 lines, -32 lines)			Patch
M	chrome/browser/password_manager/native_backend_kwallet_x.cc	View	1	3 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chromeos/dbus/cryptohome_client.cc	View		6 chunks	+6 lines, -6 lines	0 comments	Download
M	chromeos/dbus/debug_daemon_client.cc	View	1 2	1 chunk	+2 lines, -4 lines	0 comments	Download
M	chromeos/dbus/session_manager_client.cc	View		1 chunk	+2 lines, -2 lines	0 comments	Download
M	content/browser/geolocation/wifi_data_provider_linux.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	dbus/message.h	View		2 chunks	+10 lines, -7 lines	0 comments	Download
M	dbus/message.cc	View		4 chunks	+8 lines, -5 lines	0 comments	Download
M	dbus/message_unittest.cc	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	device/media_transfer_protocol/media_transfer_protocol_daemon_client.cc	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

satorux1

Thank you for fixing the const-ness! https://codereview.chromium.org/176693003/diff/30001/chromeos/dbus/debug_daemon_client.cc File chromeos/dbus/debug_daemon_client.cc (right): https://codereview.chromium.org/176693003/diff/30001/chromeos/dbus/debug_daemon_client.cc#newcode519 chromeos/dbus/debug_daemon_client.cc:519: if (!reader.PopArrayOfBytes(reinterpret_cast<const uint8**>(&buffer), ...

6 years, 10 months ago (2014-02-24 07:44:54 UTC) #2

Daniel Erat

https://codereview.chromium.org/176693003/diff/30001/chromeos/dbus/debug_daemon_client.cc File chromeos/dbus/debug_daemon_client.cc (right): https://codereview.chromium.org/176693003/diff/30001/chromeos/dbus/debug_daemon_client.cc#newcode519 chromeos/dbus/debug_daemon_client.cc:519: if (!reader.PopArrayOfBytes(reinterpret_cast<const uint8**>(&buffer), On 2014/02/24 07:44:55, satorux at chromium.org ...

6 years, 10 months ago (2014-02-24 08:16:17 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/derat@chromium.org/176693003/110001

6 years, 10 months ago (2014-02-24 08:18:50 UTC) #7

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/derat@chromium.org/176693003/110001

6 years, 10 months ago (2014-02-24 14:17:13 UTC) #9

Message was sent while issue was closed.

Change committed as 252922

Expand Messages | Collapse Messages