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

Memory Allocation for dynamic arrays in structs #123

Open
brikhoang opened this issue Apr 6, 2020 · 6 comments
Open

Memory Allocation for dynamic arrays in structs #123

brikhoang opened this issue Apr 6, 2020 · 6 comments

Comments

@brikhoang
Copy link

I was wondering if you could look at this issue:

struct book_t {
uint32_t page;
};

struct sample {
struct book_t books[10];
};

This is a bit of an unusual use case, where a dynamic array is declared inside the structure, as opposed to simply declaring a pointer to a separately initialized array. I was wondering if you could add support for this particular case? I am trying to use your library with some other pre-existing code structures which cannot change. Any help in this regard would be appreciated.

@tlsa
Copy link
Owner

tlsa commented Apr 8, 2020

This is already supported.

A schema such as:

static const cyaml_schema_field_t schema_book_fields[] = {
	CYAML_FIELD_UINT("page", CYAML_FLAG_DEFAULT,
			struct book_t, page),
	CYAML_FIELD_END
};

static const cyaml_schema_value_t schema_book_mapping = {
	CYAML_VALUE_MAPPING(CYAML_FLAG_DEFAULT,
			struct book_t, schema_book_fields),
};

static const cyaml_schema_field_t schema_sample_fields[] = {
	CYAML_FIELD_SEQUENCE_FIXED("books", CYAML_FLAG_DEFAULT,
			struct sample, books, &schema_book_mapping,
			CYAML_ARRAY_LEN(((struct sample *)NULL)->books)),
	CYAML_FIELD_END
};

static const cyaml_schema_value_t top_schema = {
	CYAML_VALUE_MAPPING(CYAML_FLAG_POINTER,
			struct sample, schema_sample_fields),
};

Loading this file:

books:
- { page: 44 }
- { page: 673 }
- { page: 5 }
- { page: 44 }
- { page: 7890 }
- { page: 30 }
- { page: 220 }
- { page: 341 }
- { page: 7 }
- { page: 51 }

With this code:

static const cyaml_config_t config = {
	.log_level = CYAML_LOG_WARNING, /* Logging errors and warnings only. */
	.log_fn = cyaml_log,            /* Use the default logging function. */
	.mem_fn = cyaml_mem,            /* Use the default memory allocator. */
};

int main(int argc, char *argv[])
{
	cyaml_err_t err;
	struct sample *s;
	enum {
		ARG_PROG_NAME,
		ARG_PATH_IN,
		ARG__COUNT,
	};

	if (argc != ARG__COUNT) {
		fprintf(stderr, "Usage:\n");
		fprintf(stderr, "  %s <INPUT>\n", argv[ARG_PROG_NAME]);
		return EXIT_FAILURE;
	}

	err = cyaml_load_file(argv[ARG_PATH_IN], &config,
			&top_schema, (cyaml_data_t **)&s, NULL);
	if (err != CYAML_OK) {
		fprintf(stderr, "ERROR: %s\n", cyaml_strerror(err));
		return EXIT_FAILURE;
	}

	for (unsigned i = 0; i < CYAML_ARRAY_LEN(s->books); i++) {
		printf("  - %"PRIu32"\n", s->books[i].page);
	}

	cyaml_free(&config, &top_schema, s, 0);

	return EXIT_SUCCESS;
}

Running it like this:

./a.out input.yaml
  - 44
  - 673
  - 5
  - 44
  - 7890
  - 30
  - 220
  - 341
  - 7
  - 51

Does that help?

@brikhoang
Copy link
Author

brikhoang commented Apr 8, 2020

Hi,
Thanks for getting back to my issue so quickly. In short, your solution does not work, as it assumes we know the size of the array beforehand. Attempting the recommended code currently thows a compile error (though this is expected, as there is no specified array size in the structure definition).

This is partially my fault for incorrectly pasting the size of the array into the sample code (I accidentally included the hardcoded size I did as a workaround). I have modified my sample to more accurately depict my problem as below:

struct book_t {
uint32_t page;
};

struct sample {
uint32_t size_of_books;
struct book_t books[];
};

Is there currently any feature in the library that supports the allocation of an array based on a previously defined field in the struct, as above? I had previously attempted to use the SEQUENCE_COUNT functionality, but realized this only allows you to specify a custom variable to store the size of array in AFTER the library has already determined the size of the array, as opposed to directly using the variable to create the array itself. Using 'size_of_books' is not necessary however if the library can determine this size from the yaml file alone.

@tlsa
Copy link
Owner

tlsa commented Apr 9, 2020

Oh, I see what you mean. There is no support for C99 flexible array members.

It would be a bit ugly to support them, because the YAML input could have the size_of_books mapping entry after the books sequence, so libcyaml could would need to reallocate it as books grew. Also there would have to be a special case where this treatment is only allowed for the last member of the struct.

@brikhoang brikhoang reopened this Apr 22, 2020
@brikhoang
Copy link
Author

Hi tlsa,

I am wondering if you've decided against supporting this problem due to its potential complications? And in that event, could you maybe point to where in the code you are handling this case so I can try to understand/modify the code myself?

Thanks!

@tlsa
Copy link
Owner

tlsa commented Apr 27, 2020

Hi,

It's still on my radar. I'll get to this after I've finished working on #90. Both will require ABI breaking changes, so it will mean making a v2.0.0 of the library.

@brikhoang
Copy link
Author

Hi, I was wondering if I could get an ETA on this?

Thanks!

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

No branches or pull requests

2 participants