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

Bottom up creation of required union with empty table fails #4

Closed
kristofferkoch opened this issue May 4, 2016 · 4 comments
Closed
Labels

Comments

@kristofferkoch
Copy link

Hi,
I have a project with a schema for messages where a union is used to include the contents of the message.

namespace UnionWithEmpty;
table RequestSomething {}
table Something { thing:uint; }
union MessageContents {RequestSomething, Something}
table Message {
    to:[uint] (required);
    from:uint;
    contents:MessageContents (required);
}
root_type Message;

However, when I try to create a RequestSomething message bottom up, I get a required assert. If i add a dummy field to RequestSomething, the example starts to work:

#include <stdio.h>
#include <assert.h>
#include "union_with_empty_builder.h"

#define ns(x) FLATBUFFERS_WRAP_NAMESPACE(UnionWithEmpty, x)
#define ARRAYSIZE(x) (sizeof(x)/sizeof(x[0]))

static size_t create_works(uint8_t *buf)
{
  flatcc_builder_t builder;
  flatcc_builder_t *B = &builder;
  flatcc_builder_init(B);

  ns(Message_start_as_root(B));
  uint32_t to[] = {1,2,3};
  uint32_t from = 5;
  ns(Message_to_create(B, to, ARRAYSIZE(to)));
  ns(Message_from_add(B, from));

  ns(RequestSomething_ref_t) req = ns(RequestSomething_create(B));
  ns(Message_contents_add(B, ns(MessageContents_as_RequestSomething(req))));
  ns(Message_end_as_root(B));

  size_t size = flatcc_builder_get_buffer_size(B);
  flatcc_builder_copy_buffer(B, buf, size);

  flatcc_builder_clear(B);
  return size;
}

static size_t create_fails(uint8_t *buf)
{
  flatcc_builder_t builder;
  flatcc_builder_t *B = &builder;
  flatcc_builder_init(B);

  ns(RequestSomething_ref_t) req = ns(RequestSomething_create(B));

  ns(Message_start_as_root(B));
  uint32_t to[] = {1,2,3};
  uint32_t from = 5;
  ns(Message_to_create(B, to, ARRAYSIZE(to)));
  ns(Message_from_add(B, from));
  ns(Message_contents_add(B, ns(MessageContents_as_RequestSomething(req))));
  ns(Message_end_as_root(B));

  size_t size = flatcc_builder_get_buffer_size(B);
  flatcc_builder_copy_buffer(B, buf, size);  
  flatcc_builder_clear(B);
  return size;
}

static void decode(const uint8_t *buf)
{
  ns(Message_table_t) dec = ns(Message_as_root(buf));
  printf("type = '%s'\n", ns(MessageContents_type_name(ns(Message_contents_type(dec)))));
}

int main()
{
  uint8_t buf[1024];
  create_works(buf);
  create_fails(buf);
  decode(buf);
  return 0;
}

Thank you for making flatcc!

@mikkelfj
Copy link
Contributor

mikkelfj commented May 4, 2016

Thanks for reporting this - I will look into it soon. Meanwhile, this is a very high quality bug report. As an example for future bug reports it can be reproduced trivially as follows:

cd /tmp
{path-to-flatcc-repo}/scripts/setup.sh -b -s issue4
cd issue4
edit src/union_with_empty.fbs (paste from above)
edit src/main.c (paste from above)
scripts/build.sh
lldb build/issue4_d
run
up
(hit return a few times)
and the offending line is listed.

(edit: since this is now fixed, check out flatcc v0.3.3 to see the example fail - though only after 0.3.3 will setup.sh link debug builds to debug flatccrt_d.a library)

@mikkelfj
Copy link
Contributor

mikkelfj commented May 4, 2016

Fixed. It has nothing to do with unions. The req reference becomes zero because table creation fails. The empty req is then considered an absent value which violates the required field requirement triggering an assertion. The req value failed as a result of an assumed allocation failure because deeper parts of the builder would not find it necessary to allocate space for table types without any fields. Now a little memory will be reserved when creating any kind of table. Struct buffers are not affected.

@mikkelfj mikkelfj closed this as completed May 4, 2016
@mikkelfj mikkelfj added the bug label May 4, 2016
@kristofferkoch
Copy link
Author

That was fixed rather quickly! Thanks a lot :)

@mikkelfj
Copy link
Contributor

This issue is now used in the following sample project:

https://github.com/dvidelabs/flatcc-meson-sample

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants