search

dylanmc / cs393_vm_api

Virtual memory abstraction & methods for constructing, accessing, manipulating and destroying address spaces
4 stars 28 forks source link

The `DataSource` API #6

Open rileyshahar opened 1 year ago

rileyshahar commented 1 year ago

I figure this is a good place to have a discussion about this API. Here's the current code, for reference: https://github.com/dylanmc/cs393_vm_api/blob/2d1299b1d4ee05cdcac73f3ef0ed3209fb9e85f1/src/data_source.rs#L4-L22 Some questions I have:

  1. Do we want to force the error type to be &str, or do we want to allow each implementer to provide their own error type? The latter is maybe slightly more complicated, but much, much more idiomatic.
  2. Why does write need mutable access to the buffer? Very possible I'm misunderstanding, but my assumption would be it just needs to read out of the buffer so it can write that data into whatever its backing memory is.
  3. Is there a reason we need to take a &mut Vec<u8> instead of &mut [u8]? My instinct is the latter is more general (since a &mut Vec can be downcast into a mutable slice, but not vice versa), but open to other considerations.

Happy to write a PR to resolve any/all of these, but wanted to raise them first, since they're more fluid design choices then my other PRs.

dylanmc commented 1 year ago

Question 1) I'm totally open to improving the Error type. I needed to choose something, but if you'd like to add something more general, go ahead and make a pull request.

2) Good catch! read needs mut access, but write does not.

3) Another good point -- one thing I haven't mentioned yet is that I'd like these APIs to take regions of memory that are multiples of the 4K page size. Your reasoning is sound wrt. &mut Vec vs. &mut [u8]. I thought the latter had fixed length -- it's a bit confusing (because [u8] without the reference would be a fixed length, right?).

rileyshahar commented 1 year ago

1) I'm totally open to improving the Error type. I needed to choose something, but if you'd like to add something more general, go ahead and make a pull request.

Interestingly, this is more complicated than I thought, because even dynamic trait objects have to specify associated types. So we can't just make an associated Error type without significantly restricting the flexibility of a MapEntry. I'll have to put more thought into it.

I thought the latter had fixed length -- it's a bit confusing (because [u8] without the reference would be a fixed length, right?)

Yeah, I guess this is a little confusing. Wish I had a good way to think about it besides just remembering it. At a high level, slices (&[T]) are just arbitrary windows into contiguous sections of memory; they don't really care whether that contiguous memory comes from an array or a vector.