Issue 9348019: Add support for byte arrays to native messages

Issue 9348019: Add support for byte arrays to native messages (Closed)

Created:
8 years, 10 months ago by Søren Gjesse

Modified:
8 years, 10 months ago

Reviewers:
siva

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Add support for byte arrays to native messages R=asiva@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=4071

Patch Set 2 : Addressed review comments from asiva@ #

Created: 8 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+160 lines, -5 lines)			Patch
M	runtime/include/dart_api.h	View	1	3 chunks	+5 lines, -1 line	0 comments	Download
M	runtime/vm/dart_api_message.h	View	1	1 chunk	+2 lines, -0 lines	0 comments	Download
M	runtime/vm/dart_api_message.cc	View	1	6 chunks	+40 lines, -4 lines	0 comments	Download
M	runtime/vm/dart_api_state.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/snapshot.cc	View		1 chunk	+13 lines, -0 lines	0 comments	Download
M	runtime/vm/snapshot_test.cc	View	1	4 chunks	+99 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

siva

LGTM http://codereview.chromium.org/9348019/diff/1/runtime/vm/dart_api_message.cc File runtime/vm/dart_api_message.cc (right): http://codereview.chromium.org/9348019/diff/1/runtime/vm/dart_api_message.cc#newcode109 runtime/vm/dart_api_message.cc:109: alloc_(NULL, 0, sizeof(Dart_CObject) + length + 1)); (sizeof(Dart_Cobject) ...

8 years, 10 months ago (2012-02-09 01:58:57 UTC) #2

Søren Gjesse

8 years, 10 months ago (2012-02-09 09:34:16 UTC) #3

https://chromiumcodereview.appspot.com/9348019/diff/1/runtime/vm/dart_api_mes...
File runtime/vm/dart_api_message.cc (right):

https://chromiumcodereview.appspot.com/9348019/diff/1/runtime/vm/dart_api_mes...
runtime/vm/dart_api_message.cc:109: alloc_(NULL, 0, sizeof(Dart_CObject) +
length + 1));
On 2012/02/09 01:58:57, asiva wrote:
> (sizeof(Dart_Cobject) + length + 1)
> 
> what is the + 1 for?
> 
> ASSERT(value != NULL);

Removed the + 1 (copy/paset from string allocation). Added ASSERT for all alloc_
calls.

https://chromiumcodereview.appspot.com/9348019/diff/1/runtime/vm/dart_api_mes...
runtime/vm/dart_api_message.cc:226: *p = Read<uint8_t>();
On 2012/02/09 01:58:57, asiva wrote:
> p[i] = Read<uint8_t>();
> 
> would avoid the need for p++

Done (for string as well).

Expand Messages | Collapse Messages