[PATCH ncns v1 0/2] ncns fixes

[PATCH ncns v1 0/2] ncns fixes

[Ammar Faizi]

Hi Al,

Two fixes this time:

1) Only take the pointer of buffer when available.

The call to `ch->ch_.read_buf()` may be undefined behavior if the
buffer hasn't been initialized. Don't touch this buffer and return
null if it's not guaranteed to be initialized.

2) Fix unused variable warning.

Fix this:

  ../chnet/chnet_node.cc: In function ‘void CHN_RingWaitCQE(const Napi::CallbackInfo&)’:
  ../chnet/chnet_node.cc:146:19: warning: variable ‘env’ set but not used [-Wunused-but-set-variable]
    146 |         Napi::Env env = info.Env();
        |                   ^~~

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (2):
  chnet: node: Only take the pointer of buffer when available
  chnet: node: Fix unused variable warning

 chnet/chnet_node.cc | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)


base-commit: 2b40ff58ef9007013ec741d2d599869e4c0b46f2
-- 
Ammar Faizi

[PATCH ncns v1 1/2] chnet: node: Only take the pointer of buffer when available

[Ammar Faizi]

The call to `ch->ch_.read_buf()` may be undefined behavior if the
buffer hasn't been initialized. Don't touch this buffer and return
null if it's not guaranteed to be initialized.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 chnet/chnet_node.cc | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/chnet/chnet_node.cc b/chnet/chnet_node.cc
index 1491fd8..c89a745 100644
--- a/chnet/chnet_node.cc
+++ b/chnet/chnet_node.cc
@@ -267,20 +267,18 @@ static void CHN_NetSetURL(const Napi::CallbackInfo &info)
 	ch->ch_.SetURL(url.c_str());
 }
 
-static Napi::String CHN_NetReadBuf(const Napi::CallbackInfo &info)
+static Napi::Value CHN_NetReadBuf(const Napi::CallbackInfo &info)
 {
 	Napi::Env env = info.Env();
-	const char *buf;
 	CHNetNode *ch;
 	int ret;
 
 	ch = (CHNetNode *)info.Data();
-	buf = ch->ch_.read_buf();
 	ret = ch->ch_.read_ret();
 	if (ret > 0)
-		return Napi::String::New(env, buf, (size_t)ret);
+		return Napi::String::New(env, ch->ch_.read_buf(), (size_t)ret);
 
-	return Napi::String::New(env, "");
+	return env.Null();
 }
 
 static Napi::Number CHN_NetReadRet(const Napi::CallbackInfo &info)
-- 
Ammar Faizi

[PATCH ncns v1 2/2] chnet: node: Fix unused variable warning

[Ammar Faizi]

Fix this:

  ../chnet/chnet_node.cc: In function ‘void CHN_RingWaitCQE(const Napi::CallbackInfo&)’:
  ../chnet/chnet_node.cc:146:19: warning: variable ‘env’ set but not used [-Wunused-but-set-variable]
    146 |         Napi::Env env = info.Env();
        |                   ^~~

Fixes: 4fc27d45cfbb5a19a499ca4398b522038cbd8e42 ("chnet_node: Fully integrate chnet ring with NodeJS")
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 chnet/chnet_node.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/chnet/chnet_node.cc b/chnet/chnet_node.cc
index c89a745..06f624e 100644
--- a/chnet/chnet_node.cc
+++ b/chnet/chnet_node.cc
@@ -143,7 +143,6 @@ static Napi::Value CHN_RingSubmitSQE(const Napi::CallbackInfo &info)
 
 static void CHN_RingWaitCQE(const Napi::CallbackInfo &info)
 {
-	Napi::Env env = info.Env();
 	uint32_t to_wait;
 	CNRingCtx *ring;
 
-- 
Ammar Faizi

Re: [PATCH ncns v1 0/2] ncns fixes

[Alviro Iskandar Setiawan]

On Wed, Aug 17, 2022 at 8:09 AM Ammar Faizi wrote:
> Hi Al,
>
> Two fixes this time:

applied

tq

-- Viro

[PATCH ncns v1 0/2] ncns fixes

[Ammar Faizi]

Hi Al,

Yet another small series for ncns.

1) Initialize `read_ret_` to zero.

We can't rely on read_ret_ value being random at initialization. This
caused a real crash where @ret value being random (83382480 in this
case):

  ret = 83382480
  [0817/093957.227555:FATAL:scoped_refptr.h(272)] Check failed: ptr_.
  #0 0x7fa9a8a6b62c <unknown>
  #1 0x7fa9a87cb30a <unknown>
  #2 0x7fa9a87cb2c5 <unknown>
  #3 0x7fa9a881c799 <unknown>
  #4 0x7fa9a881cfa9 <unknown>
  #5 0x7fa9a878537b <unknown>
  #6 0x7fa9a93866b6 <unknown>
  #7 0x7fa9a93121fa <unknown>
  #8 0x7fa9a931debd <unknown>
  #9 0x000000b0deed <unknown>
  #10 0x000000d972d0 <unknown>
  #11 0x000000d9880f <unknown>
  #12 0x0000016dabb9 <unknown>

2) Set error string when the Chromium hits net error.

Translate the error code to string when the Chromium hits a net error.
This makes debugging easier for the callers because now we can know
what the returned error code means, easily.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
Ammar Faizi (2):
  chnet: Initialize `read_ret_` to zero
  chnet: Set error string when the Chromium hits net error

 chnet/chnet.cc | 12 ++++++++++++
 chnet/chnet.h  |  2 ++
 2 files changed, 14 insertions(+)


base-commit: 971f49f53c730aa82466b251a2b682c0827c3ec4
-- 
Ammar Faizi

Re: [PATCH ncns v1 0/2] ncns fixes

[Alviro Iskandar Setiawan]

On Wed, Aug 17, 2022 at 8:35 PM Ammar Faizi wrote:
> Hi Al,
>
> Yet another small series for ncns.

applied

tq

-- Viro