mnemonikr / symbolic-pcode

Apache License 2.0
0 stars 0 forks source link

Memory tree read and write functions do not match associated memory function names #77

Closed mnemonikr closed 8 months ago

mnemonikr commented 1 year ago

Memory tree uses read and write but memory uses read_bytes_owned and write_bytes.

https://github.com/mnemonikr/pcode/blob/9694e9ba6563ce5cc328aa2d97d406a28db0516a/src/mem.rs#L298-L299

https://github.com/mnemonikr/pcode/blob/9694e9ba6563ce5cc328aa2d97d406a28db0516a/src/mem.rs#L336-L337

  1. We should standardize on a single API. The names read and write seem simplest. The _bytes is superfluous since the VarnodeData::size indicates how much should be read or written, and this field is in bytes.
  2. Nearly all calls ultimately call read_bytes_owned, so it's questionable if the sister API read_bytes is necessary. The non-owned reads occur when reading values which cannot be symbolic (e.g. reading memory to load instructions). Seems a better API would be read_concrete which internally could avoid cloning the data prior to concretization.
mnemonikr commented 8 months ago

Resolved by #87 which renamed read_bytes_owned to read and removed read_bytes.