Clarify memory guarantees of deno_buf

3 files changed, 17 insertions, 21 deletions

diff --git a/deno2/deno.cc b/deno2/deno.cc
index 7b84abf22..caea85ffb 100644
--- a/deno2/deno.cc
+++ b/deno2/deno.cc
@@ -124,7 +124,6 @@ void Sub(const v8::FunctionCallbackInfo<v8::Value>& args) {
   d->sub.Reset(isolate, func);
 }
 
-// Called from JavaScript, routes message to golang.
 void Pub(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
   Deno* d = static_cast<Deno*>(isolate->GetData(0));
@@ -145,10 +144,12 @@ void Pub(const v8::FunctionCallbackInfo<v8::Value>& args) {
   auto ab = v8::Local<v8::ArrayBuffer>::Cast(ab_v);
   auto contents = ab->GetContents();
 
-  void* buf = contents.Data();
-  int buflen = static_cast<int>(contents.ByteLength());
+  // data is only a valid pointer until the end of this call.
+  const char* data =
+      const_cast<const char*>(reinterpret_cast<char*>(contents.Data()));
+  deno_buf buf{data, contents.ByteLength()};
 
-  auto retbuf = d->cb(d, channel, deno_buf{buf, buflen});
+  auto retbuf = d->cb(d, channel, buf);
   if (retbuf.data) {
     // TODO(ry) Support zero-copy.
     auto ab = v8::ArrayBuffer::New(d->isolate, retbuf.len);
@@ -292,7 +293,7 @@ bool deno_pub(Deno* d, const char* channel, deno_buf buf) {
     return false;
   }
 
-  // TODO(ry) support zero copy.
+  // TODO(ry) support zero-copy.
   auto ab = v8::ArrayBuffer::New(d->isolate, buf.len);
   memcpy(ab->GetContents().Data(), buf.data, buf.len);
 
diff --git a/deno2/include/deno.h b/deno2/include/deno.h
index 63f26ba8c..b8d4e59a4 100644
--- a/deno2/include/deno.h
+++ b/deno2/include/deno.h
@@ -10,14 +10,16 @@ extern "C" {
 
 // Data that gets transmitted.
 typedef struct {
-  void* data;
+  const char* data;
   size_t len;
 } deno_buf;
 
 struct deno_s;
 typedef struct deno_s Deno;
 
-// The callback from V8 when data is sent.
+// A callback to receive a message from deno_pub javascript call.
+// buf is valid only for the lifetime of the call.
+// The returned deno_buf is returned from deno_pub in javascript.
 typedef deno_buf (*deno_sub_cb)(Deno* d, const char* channel, deno_buf buf);
 
 void deno_init();
diff --git a/deno2/mock_runtime_test.cc b/deno2/mock_runtime_test.cc
index 297b58b22..929793645 100644
--- a/deno2/mock_runtime_test.cc
+++ b/deno2/mock_runtime_test.cc
@@ -23,10 +23,7 @@ TEST(MockRuntimeTest, ErrorsCorrectly) {
   deno_dispose(d);
 }
 
-deno_buf strbuf(const char* str) {
-  void* d = reinterpret_cast<void*>(const_cast<char*>(str));
-  return deno_buf{d, strlen(str)};
-}
+deno_buf strbuf(const char* str) { return deno_buf{str, strlen(str)}; }
 
 TEST(MockRuntimeTest, PubSuccess) {
   Deno* d = deno_new(NULL, NULL);
@@ -56,11 +53,9 @@ TEST(MockRuntimeTest, SubReturnEmpty) {
     count++;
     EXPECT_STREQ(channel, "SubReturnEmpty");
     EXPECT_EQ(static_cast<size_t>(3), buf.len);
-    // TODO(ry) buf.data should just be a char*.
-    char* data = reinterpret_cast<char*>(buf.data);
-    EXPECT_EQ(data[0], 'a');
-    EXPECT_EQ(data[1], 'b');
-    EXPECT_EQ(data[2], 'c');
+    EXPECT_EQ(buf.data[0], 'a');
+    EXPECT_EQ(buf.data[1], 'b');
+    EXPECT_EQ(buf.data[2], 'c');
     return deno_buf{nullptr, 0};
   });
   EXPECT_TRUE(deno_execute(d, "a.js", "SubReturnEmpty()"));
@@ -74,11 +69,9 @@ TEST(MockRuntimeTest, SubReturnBar) {
     count++;
     EXPECT_STREQ(channel, "SubReturnBar");
     EXPECT_EQ(static_cast<size_t>(3), buf.len);
-    // TODO(ry) buf.data should just be a char*.
-    char* data = reinterpret_cast<char*>(buf.data);
-    EXPECT_EQ(data[0], 'a');
-    EXPECT_EQ(data[1], 'b');
-    EXPECT_EQ(data[2], 'c');
+    EXPECT_EQ(buf.data[0], 'a');
+    EXPECT_EQ(buf.data[1], 'b');
+    EXPECT_EQ(buf.data[2], 'c');
     return strbuf("bar");
   });
   EXPECT_TRUE(deno_execute(d, "a.js", "SubReturnBar()"));