This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9107

hg-core: return Err if `offset != bytes.len()`
ClosedPublic

Authored by acezar on Sep 28 2020, 9:49 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG1cef583541c0: hg-core: return Err if `offset != bytes.len()`
Summary

(D8958#inline-14994 followup 2/2)

  • make Index owner of its bytes
  • make Index::new return an error if offset != bytes.len()

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

acezar created this revision.Sep 28 2020, 9:49 AM
Alphare accepted this revision.Sep 28 2020, 10:32 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/revlog/revlog.rs
61

Is this comment still relevant?

acezar retitled this revision from hg-core: return Err if `offset != bytes.len()` (D8958#inline-14994 followup 2/2) to hg-core: return Err if `offset != bytes.len()`.Sep 29 2020, 11:53 AM
acezar edited the summary of this revision. (Show Details)
acezar updated this revision to Diff 22921.Sep 29 2020, 1:29 PM
acezar marked an inline comment as done.Sep 29 2020, 1:29 PM
acezar added inline comments.
rust/hg-core/src/revlog/revlog.rs
61

Not sure.

Antoine cezar <acezar@chwitlabs.fr> added a commit: rHG1cef583541c0: hg-core: return Err if `offset != bytes.len()`.Sep 29 2020, 1:52 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG1cef583541c0: hg-core: return Err if `offset != bytes.len()` (authored by Antoine cezar <acezar@chwitlabs.fr>). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 22936

rust/hg-core/src/revlog/index.rs

use std::ops::Deref; use std::ops::Deref;
use byteorder::{BigEndian, ByteOrder}; use byteorder::{BigEndian, ByteOrder};
use crate::revlog::revlog::RevlogError;
use crate::revlog::{Revision, NULL_REVISION}; use crate::revlog::{Revision, NULL_REVISION};
pub const INDEX_ENTRY_SIZE: usize = 64; pub const INDEX_ENTRY_SIZE: usize = 64;
/// A Revlog index /// A Revlog index
pub struct Index { pub struct Index {
bytes: Box<dyn Deref<Target = [u8]> + Send>, bytes: Box<dyn Deref<Target = [u8]> + Send>,
/// Offsets of starts of index blocks. /// Offsets of starts of index blocks.
/// Only needed when the index is interleaved with data. /// Only needed when the index is interleaved with data.
offsets: Option<Vec<usize>>, offsets: Option<Vec<usize>>,
} }
impl Index { impl Index {
/// Create an index from bytes. /// Create an index from bytes.
/// Calculate the start of each entry when is_inline is true. /// Calculate the start of each entry when is_inline is true.
pub fn new(bytes: Box<dyn Deref<Target = [u8]> + Send>) -> Self { pub fn new(
bytes: Box<dyn Deref<Target = [u8]> + Send>,
) -> Result<Self, RevlogError> {
if is_inline(&bytes) { if is_inline(&bytes) {
let mut offset: usize = 0; let mut offset: usize = 0;
let mut offsets = Vec::new(); let mut offsets = Vec::new();
while offset + INDEX_ENTRY_SIZE <= bytes.len() { while offset + INDEX_ENTRY_SIZE <= bytes.len() {
offsets.push(offset); offsets.push(offset);
let end = offset + INDEX_ENTRY_SIZE; let end = offset + INDEX_ENTRY_SIZE;
let entry = IndexEntry { let entry = IndexEntry {
bytes: &bytes[offset..end], bytes: &bytes[offset..end],
offset_override: None, offset_override: None,
}; };
offset += INDEX_ENTRY_SIZE + entry.compressed_len(); offset += INDEX_ENTRY_SIZE + entry.compressed_len();
} }
Self { if offset == bytes.len() {
Ok(Self {
bytes, bytes,
offsets: Some(offsets), offsets: Some(offsets),
})
} else {
Err(RevlogError::Corrupted)
} }
} else { } else {
Self { Ok(Self {
bytes, bytes,
offsets: None, offsets: None,
} })
} }
} }
/// Value of the inline flag. /// Value of the inline flag.
pub fn is_inline(&self) -> bool { pub fn is_inline(&self) -> bool {
is_inline(&self.bytes) is_inline(&self.bytes)
} }

rust/hg-core/src/revlog/revlog.rs

let index_mmap = let index_mmap =
mmap_open(&index_path).map_err(RevlogError::IoError)?; mmap_open(&index_path).map_err(RevlogError::IoError)?;
let version = get_version(&index_mmap); let version = get_version(&index_mmap);
if version != 1 { if version != 1 {
return Err(RevlogError::UnsuportedVersion(version)); return Err(RevlogError::UnsuportedVersion(version));
} }
let index = Index::new(Box::new(index_mmap)); let index = Index::new(Box::new(index_mmap))?;
// TODO load data only when needed // // TODO load data only when needed //
AlphareUnsubmitted
Not Done

Is this comment still relevant?

Alphare: Is this comment still relevant?
acezarAuthorUnsubmitted
Done

Not sure.

acezar: Not sure.
// type annotation required // type annotation required
// won't recognize Mmap as Deref<Target = [u8]> // won't recognize Mmap as Deref<Target = [u8]>
let data_bytes: Option<Box<dyn Deref<Target = [u8]> + Send>> = let data_bytes: Option<Box<dyn Deref<Target = [u8]> + Send>> =
if index.is_inline() { if index.is_inline() {
None None
} else { } else {
let data_path = index_path.with_extension("d"); let data_path = index_path.with_extension("d");
let data_mmap = let data_mmap =