Skip to content

Dgram buffers #13623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DamienOReilly wants to merge 13 commits into from
Closed

Dgram buffers #13623

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9d6cd42
doc: added docs for send/receive buffer sizes
DamienOReilly Jun 11, 2017
b84bf95
dgram: support for setting socket buffer size
DamienOReilly Jun 11, 2017
8aed7aa
dgram: tests for setting socket buffer size
DamienOReilly Jun 11, 2017
f7c1aa6
src: udp - support to set socket buffer sizes
DamienOReilly Jun 11, 2017
a99ec11
dgram: fixed indentation
DamienOReilly Jun 11, 2017
2581c09
dgram: added method mapping
DamienOReilly Jun 11, 2017
b0ac4ff
dgram: fixed test for os independence. fixed docs
DamienOReilly Jun 12, 2017
f6bf7f2
dgram: fixed syntax, added options support
DamienOReilly Jun 12, 2017
d5789c0
dgram: add support to get buffer sizes
DamienOReilly Jun 14, 2017
cd79a58
dgram: fixed typos in tests and docs
DamienOReilly Jun 14, 2017
de458d1
dgram: lint errors and other pr comments
DamienOReilly Jun 14, 2017
f196be9
dgram: addressing pr coments
DamienOReilly Jun 21, 2017
f4d3564
dgram: fixed buffer sizes in tests
DamienOReilly Sep 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
src: udp - support to set socket buffer sizes
  • Loading branch information
@DamienOReilly
DamienOReilly committed Sep 10, 2017
commit f7c1aa66653a4407d714fec80a381372380d19c3
30 changes: 30 additions & 0 deletions src/udp_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,36 @@ void UDPWrap::Bind6(const FunctionCallbackInfo<Value>& args) {
}


void UDPWrap::SetRecvBufferSize(const FunctionCallbackInfo<Value>& args) {
UDPWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap,
args.Holder(),
args.GetReturnValue().Set(UV_EBADF));

CHECK(args[0]->IsUint32());
Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't want to crash when the user does socket.setRecvBufferSize(Infinity). Throwing a TypeError should be a better choice.

Copy link
Copy Markdown
Contributor

@refack refack Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimothyGu you mean on the JS side, right?

Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Copy Markdown
Contributor Author

@DamienOReilly DamienOReilly Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted, do you recommend:

  if (!args[0]->IsUint32()) {
    return env->ThrowTypeError("size must be an uint32");
  }

Copy link
Copy Markdown
Member

@jasnell jasnell Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend doing the type check in JS. Just something simple like:

const errors = require('internal/errors');

// then within the the functions
if (typeof size !== 'number')
  throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'size', 'number');

Copy link
Copy Markdown
Contributor Author

@DamienOReilly DamienOReilly Jun 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your input. Do you think I need to do any safety checking in udp_wrap.cc SetRecvBufferSize() or we assume only dgram.js will interact with udp_wrap ?

Copy link
Copy Markdown
Member

@jasnell jasnell Jun 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, once the type checking is added to the js side, I would keep the CHECK in place on the native side. That should only be hit if we do something wrong in core or if the user is doing something they really shouldn't.


int inputSize = args[0]->Uint32Value();
Copy link
Copy Markdown
Member

@TimothyGu TimothyGu Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uint32Value without v8::Context specified is deprecated. Use args[0]->Uint32Value(info.GetIsolate()->GetCurrentContext()).

Copy link
Copy Markdown
Contributor

@cjihrig cjihrig Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we generally prefer camelCase for JavaScript and snake case for C++.

Copy link
Copy Markdown
Contributor Author

@DamienOReilly DamienOReilly Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, will see to this.

Copy link
Copy Markdown
Member

@addaleax addaleax Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use args[0]->Uint32Value(info.GetIsolate()->GetCurrentContext()).

That’s not necessary if you already checked args[0]->IsUint32(),args[0].As<Uint32>()->Value() should do instead.

int err = uv_recv_buffer_size(reinterpret_cast<uv_handle_t*>(&wrap->handle_), &inputSize);

args.GetReturnValue().Set(err);
}


void UDPWrap::SetSendBufferSize(const FunctionCallbackInfo<Value>& args) {
UDPWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap,
args.Holder(),
args.GetReturnValue().Set(UV_EBADF));

CHECK(args[0]->IsUint32());

int inputSize = args[0]->Uint32Value();
int err = uv_send_buffer_size(reinterpret_cast<uv_handle_t*>(&wrap->handle_), &inputSize);

args.GetReturnValue().Set(err);
}


#define X(name, fn) \
void UDPWrap::name(const FunctionCallbackInfo<Value>& args) { \
UDPWrap* wrap = Unwrap<UDPWrap>(args.Holder()); \
Expand Down
2 changes: 2 additions & 0 deletions src/udp_wrap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class UDPWrap: public HandleWrap {
const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetBroadcast(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetTTL(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetRecvBufferSize(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetSendBufferSize(const v8::FunctionCallbackInfo<v8::Value>& args);

static v8::Local<v8::Object> Instantiate(Environment* env, AsyncWrap* parent);
uv_udp_t* UVHandle();
Expand Down