Skip to content

Conversation

@dryajov
Copy link
Contributor

@dryajov dryajov commented Sep 15, 2023

No description provided.

@dryajov
Copy link
Contributor Author

dryajov commented Sep 15, 2023

Added some tests and a very ugly locking mechanism that we can hopefully refine. The good thing is that all tests pass and there are 0 changes to the api.

@elcritch
Copy link
Contributor

Added some tests and a very ugly locking mechanism that we can hopefully refine. The good thing is that all tests pass and there are 0 changes to the api.

Ok, nice.

result = newSeq[byte](self.len)
if self.len() > 0:
copyMem(addr result[0], unsafeAddr self[].buf[0], self.len)
copyMem(addr result[0], addr self[].buf[0], self.len)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah good! I was thinking about that this weekend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, but maybe this should also be switched to baseAddr? Not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or the other way around, switch baseAddr to addr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imo, addr is probably safer, baseAddr does a straight up cast.

@dryajov
Copy link
Contributor Author

dryajov commented Sep 20, 2023

This logos-storage/logos-storage-nim#552 works now.

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.

3 participants