Skip to content
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.

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

[wip] Parse args #8

Merged
merged 9 commits into from
Oct 11, 2017
Merged

[wip] Parse args #8

merged 9 commits into from
Oct 11, 2017

Conversation

mapsam
Copy link
Contributor

@mapsam mapsam commented Oct 10, 2017

This sets up the vtquery.cpp file for parsing arguments before heading into the worker.

TODO

  • write validator for the first argument, array of buffer objects

@mapsam mapsam changed the title Parse args [wip] Parse args Oct 10, 2017
src/vtquery.cpp Outdated
if (!geometry_val->IsString())
return utils::CallbackError("'geometry' option must be a string", callback);

Nan::Utf8String geometry_utf8_value(geometry_val);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is ideal to check for geometry_utf8_value.length() <= 0 before trying to create a std::string since passing a null string may crash unpredictably (refs mapbox/node-fontnik#124)

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, while std::string s(nullptr); crashes, std::string s(nullptr,0); does not. So technically this usage is safe. However, it is still ideal to catch geometry_utf8_value.length() <= 0 since a zero length string would be an invalid option value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@springmeyer I originally had this check, but since a couple lines down we are checking for string values == point linestring polygon and exiting if they don't, I figured this null string couldn't get too far. Happy to add it back in though!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 on adding back.

src/vtquery.cpp Outdated
if (num_buffers <= 0)
return utils::CallbackError("'buffers' array must be of length greater than 0", callback);

// std::vector<> - some sort of storage mechanism for buffers and their zxy values here
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@springmeyer any idea what a good data structure would be here? A vector that contains maps of buffer, x, y, z values? Like:

std::vector<std::map<...>>

Copy link
Contributor Author

@mapsam mapsam Oct 11, 2017

Choose a reason for hiding this comment

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

Or perhaps we create a custom struct that has these values defined, so we could do a vector of structs with the proper values? Something like

std::vector<buf> bufs;

struct buf {
  int z;
  int x;
  int y;
  node::Buffer buffer;
}

// start doing stuff
buf buffer_obj;
buffer_obj.z = 0;
buffer_obj.x = 0;
buffer_obj.y = 0;
node::Buffer real_buffer;
real_buffer = node::Buffer::Data(buffer_data); // defined elsewhere
buffer_obj.buffer = real_buffer;
// stop doing stuff
bufs.push_back(buffer_obj);

@mapsam mapsam merged commit d92efd1 into master Oct 11, 2017
@mapsam mapsam deleted the parse-args branch October 11, 2017 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants