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

CBOR/MessagePack from uint8_t * and size #478

Closed
pboettch opened this issue Feb 28, 2017 · 8 comments
Closed

CBOR/MessagePack from uint8_t * and size #478

pboettch opened this issue Feb 28, 2017 · 8 comments
Assignees
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON kind: enhancement/improvement solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@pboettch
Copy link
Contributor

How would one use from_cbor/from_messagepack when data is stored in a uint8_t-buffer received via a low-level C library?

Could something be done in this library to support this without doing copy of the data into a vector?

@theodelrieu
Copy link
Contributor

I think those methods should behave like parse, to be more generic indeed

@pboettch
Copy link
Contributor Author

At a first look I thought that simply replacing the vector as argument and giving a uint8_t *and a size_t could work with the existing functions. On a second glance, I think this needs to be tried ;-) .

@nlohmann
Copy link
Owner

nlohmann commented May 8, 2017

I have a question regarding this issue.

For the default JSON parser, we currently have a lot of overloads:

  • T (&array)[N]
  • const CharT (CharT is a pointer to a 1-byte integral type)
  • std::istream& and std::istream&&
  • IteratorType first, IteratorType last
  • const ContiguousContainer& (container with contiguous storage of 1-byte values)

Internally, this is realized with an input adapter class, see https:/nlohmann/json/blob/develop/src/json.hpp#L8708.

When I now add these overloads to the CBOR and MessagePack parsers, this would mean 8 additional functions which are more or less the same, because each chooses one of the input adapters, which is then used to configure a binary reader (https:/nlohmann/json/blob/develop/src/json.hpp#L8954) which takes care of the acutal parsing.

Before I start copying/pasting, I was wondering if there was a more elegant solution. A first idea would be to implement some kind of

template<class ... Types>
static basic_json from_cbor(Types ... args)

which then forwards args to the constructors of the input_adapter class and basically allowing to read CBOR from anything that can create an input adapter.

This would have the advantage of only two functions (one for CBOR and one for MessagePack), but would have the disadvantage of being a bit obscure for the users of the library, because they cannot immediately recognize which arguments can be used. Of course, this could be tackled by extensive documentation, but still.

Another disadvantage of this approach is the fact that it cannot be used to limit the number of parse overloads, because these all have a const parser_callback_t cb = nullptr as last parameter which cannot follow a parameter pack.

Anyway, what are your thoughts about this?

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label May 8, 2017
@Type1J
Copy link
Contributor

Type1J commented Jun 1, 2017

I'm not sure what the status of it is, but std::array_view is proposed. I think that this is a task well suited to it.

@nlohmann
Copy link
Owner

nlohmann commented Jun 1, 2017

std::array_view is not part of C++11, so unfortunately it won't be applicable to this library for a while.

@nlohmann
Copy link
Owner

Opened #623 as place to discuss the structure of the parsing functions.

nlohmann added a commit that referenced this issue Jul 23, 2017
- You can now pass a reference to a vector to the to_cbor and to_msgpack functions. The output will be written (appended) to the vector. #476

- You can now pass an output stream with uint8_t character type to the to_cbor and to_msgpack functions. #477

- You can now read from uint8_t */size in the to_cbor and to_msgpack functions. An input adapter will be created from this pair, so you need to use braces. #478
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Jul 23, 2017
@nlohmann
Copy link
Owner

Function

static basic_json from_cbor(detail::input_adapter i)
{
    return binary_reader(i).parse_cbor();
}

now allows to read CBOR (and similarly, MessagePack) to a lot of types from which an input_adapter can be constructed. This includes a uint8_t */size_t pair.

For a std::vector<uint8_t> packed, the call is

json::from_cbor({packed.data(), packed.size()}

(Note the braces around the pair.)

@pboettch
Copy link
Contributor Author

Great. And no copy of the buffer. Thanks. Can't wait for a release ;-) .

@nlohmann nlohmann removed the state: please discuss please discuss the issue or vote for your favorite option label Jul 24, 2017
@nlohmann nlohmann added this to the Release 3.0.0 milestone Jul 24, 2017
@nlohmann nlohmann self-assigned this Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON kind: enhancement/improvement solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

4 participants