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

Function tables shouldn't be required #21

Open
3 tasks
LeoVen opened this issue Feb 22, 2023 · 3 comments
Open
3 tasks

Function tables shouldn't be required #21

LeoVen opened this issue Feb 22, 2023 · 3 comments
Labels
code Issues related to code enhancement New feature or request

Comments

@LeoVen
Copy link
Owner

LeoVen commented Feb 22, 2023

The library should:

  • Allow fkey and fval parameters to be null
  • Check in _new() for required functions
  • Check if the struct and if the functions are null before every usage and report errors instead of crashing
@LeoVen LeoVen added enhancement New feature or request code Issues related to code labels Feb 23, 2023
@duarm
Copy link

duarm commented Nov 2, 2023

Fell for this one, I thought f_val was copied into the list, not kept as a reference, so I did something like this

element_new_custom(50, &((struct tile_effects_list_fval){ .cmp = compare_element }), NULL, NULL);

and you can guess what happened

@LeoVen
Copy link
Owner Author

LeoVen commented Dec 11, 2023

Fell for this one, I thought f_val was copied into the list, not kept as a reference, so I did something like this

element_new_custom(50, &((struct tile_effects_list_fval){ .cmp = compare_element }), NULL, NULL);

and you can guess what happened

Now I'm really starting to think if the function tables should be kept as a copy for each ds instead of a pointer

@duarm
Copy link

duarm commented Dec 11, 2023

Not sure about the memory / indirect call tradeoff, it seems worth it to just copy the table since it will be most likely on the cache.

on my own modified version, I removed the default allocation table and made it required to be consistent with fval, and one of the problems I noticed is that most libs expect a different table e.g. some expect { .free, .malloc }, while others expect { .realloc, .malloc }, keeping a table for all of them around is annoying. So you might consider copying the allocation table too for consistency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Issues related to code enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants