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

Added LIST_allocator API #74

Closed
wants to merge 2 commits into from
Closed

Added LIST_allocator API #74

wants to merge 2 commits into from

Conversation

khushal-banks
Copy link
Contributor

This API is used in LIST_cons_gl() to allocate list memory.
Also, added error handling logic for this API.

Now, LIST_DEFAULT_ALLOCF macro can be used to replace memory allocation function for LIST header (default value: malloc)

These changes are an attempt to resolve #69

Modified LIST_HEADER_VERSION to 0.1.1

This API is used in LIST_cons_gl() to allocate list memory. Also, added error handling logic for this API
Now, LIST_DEFAULT_ALLOCF macro can be used to replace memory allocation function for LIST header (default value: malloc)
Modified LIST_HEADER_VERSION to 0.1.1
Copy link
Owner

@jgabaut jgabaut left a comment

Choose a reason for hiding this comment

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

The changes are well meant and make sense, but I can't accept this as of now because I don't yet know how I'd prep the new API.

I still haven't considered many options, like:

  • Should the custom definition override malloc for the _gl() family? (In this PR: yes)

  • Should a separate set with a different suffix be added? (In this PR: no)

  • Should it use a predefinition macro so that you can customise it at time of definition? (In this PR: missing the ifndef )

  • Should it be customisable per-list, or per-program? (In this PR: per list)

This change is not planned for very soon. At the moment, I'm not sure how to proceed on this. I hope you can understand.

@khushal-banks
Copy link
Contributor Author

khushal-banks commented Apr 30, 2024

The changes are well meant and make sense, but I can't accept this as of now because I don't yet know how I'd prep the new API

No problem. Just close this as invalid or not-planned.
Please, Don't keep it hanging as open.

Also, I am not sure if following in this PR were necessary:

#define LIST_allocator LIST_IMPL(allocator)
#undef LIST_allocator

This change is not planned for very soon. At the moment, I'm not sure how to proceed on this. I hope you can understand.

Yes i can understand. I was also feeling that something is missing while submitting this PR.

@jgabaut
Copy link
Owner

jgabaut commented Apr 30, 2024

Thanks for understanding. In a close future when the wanted API design is more clear, we can work this out.

@jgabaut jgabaut closed this Apr 30, 2024
@khushal-banks khushal-banks deleted the list_allocator_fn branch April 30, 2024 10:45
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.

[FEATURE] Add support for custom allocation functions in generated list header
2 participants