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

undefined behavior in grow() #9

Open
phillipwood opened this issue Mar 8, 2022 · 2 comments
Open

undefined behavior in grow() #9

phillipwood opened this issue Mar 8, 2022 · 2 comments

Comments

@phillipwood
Copy link

I saw the link to this project on hacker news, it looks interesting. One thing I noticed was that in grow() it has

// TYPE1 -> TYPE4
RELOC(4)
memcpy (newdata, DATA(newhead, TYPE1), dims.len+1); 

the result of memcpy() is undefined when the source and destination overlap, this should use memmove() instead. One a related note the code to grow a string seems to be duplicated in a different form in resize(), perhaps that could call grow() instead.

@alcover
Copy link
Owner

alcover commented Mar 8, 2022

Hello
I didn't consider realloc may simply expand in-place. Nice find !
I'll look into it.

About reusing grow(), it's specialized in increase-only.
But you're right the code needs maybe refactoring.

If you wanna PR or be a contributor, I'd be glad.
A new simpler version is coming though. Maybe we'll see later ?

Thx a lot

@alcover
Copy link
Owner

alcover commented Jun 3, 2022

Phillip, i've been busy all this time but I'll work on your find soon.

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