This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7921

rust-dirs-multiset: improve temporary error message
ClosedPublic

Authored by Alphare on Jan 17 2020, 5:41 AM.

Details

Reviewers
kevincox
pulkit
Group Reviewers
hg-reviewers
Commits
rHG934a79697c36: rust-dirs-multiset: improve temporary error message
rHG5a07db9a31fd: rust-dirs-multiset: improve temporary error message
rHGa5c7d6cdaa2e: rust-dirs-multiset: improve temporary error message
Summary

While we wait on a future patch that could verify that the paths passed to
DirsMultiset have been audited, we still need to handle this error.
This patch makes it easier to bubble up and makes the error clearer.

Also, this patch introduces the subslice_index function that could be useful
for other - albeit niche - purposes.

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

Alphare created this revision.Jan 17 2020, 5:41 AM
kevincox requested changes to this revision.Jan 24 2020, 4:59 AM
kevincox added inline comments.
rust/hg-core/src/dirstate/dirs_multiset.rs
84

It appears to me that find_dirs always returns prefixes. Why is this offset required?

This revision now requires changes to proceed.Jan 24 2020, 4:59 AM
Alphare marked an inline comment as done.Jan 24 2020, 5:16 AM
martinvonz added a subscriber: martinvonz.Jan 24 2020, 11:47 AM

It seems this one doesn't apply cleanly

pulkit accepted this revision.Feb 7 2020, 4:10 PM
Alphare added a commit: rHGa5c7d6cdaa2e: rust-dirs-multiset: improve temporary error message.Feb 7 2020, 4:38 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGa5c7d6cdaa2e: rust-dirs-multiset: improve temporary error message (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 20002

rust/hg-core/src/dirstate/dirs_multiset.rs

// dirs_multiset.rs // dirs_multiset.rs
// //
// Copyright 2019 Raphaël Gomès <rgomes@octobus.net> // Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
//! A multiset of directory names. //! A multiset of directory names.
//! //!
//! Used to counts the references to directories in a manifest or dirstate. //! Used to counts the references to directories in a manifest or dirstate.
use crate::{ use crate::{
dirstate::EntryState, dirstate::EntryState,
utils::{ utils::{
files, files,
hg_path::{HgPath, HgPathBuf}, hg_path::{HgPath, HgPathBuf, HgPathError},
}, },
DirstateEntry, DirstateMapError, FastHashMap, DirstateEntry, DirstateMapError, FastHashMap,
}; };
use std::collections::{hash_map, hash_map::Entry, HashMap, HashSet}; use std::collections::{hash_map, hash_map::Entry, HashMap, HashSet};
// could be encapsulated if we care API stability more seriously // could be encapsulated if we care API stability more seriously
pub type DirsMultisetIter<'a> = hash_map::Keys<'a, HgPathBuf, u32>; pub type DirsMultisetIter<'a> = hash_map::Keys<'a, HgPathBuf, u32>;
pub fn add_path( pub fn add_path(
&mut self, &mut self,
path: impl AsRef<HgPath>, path: impl AsRef<HgPath>,
) -> Result<(), DirstateMapError> { ) -> Result<(), DirstateMapError> {
for subpath in files::find_dirs(path.as_ref()) { for subpath in files::find_dirs(path.as_ref()) {
if subpath.as_bytes().last() == Some(&b'/') { if subpath.as_bytes().last() == Some(&b'/') {
// TODO Remove this once PathAuditor is certified // TODO Remove this once PathAuditor is certified
// as the only entrypoint for path data // as the only entrypoint for path data
return Err(DirstateMapError::ConsecutiveSlashes); let second_slash_index = subpath.len() - 1;
return Err(DirstateMapError::InvalidPath(
HgPathError::ConsecutiveSlashes {
kevincoxUnsubmitted
Done

It appears to me that find_dirs always returns prefixes. Why is this offset required?

kevincox: It appears to me that `find_dirs` always returns prefixes. Why is this offset required?
bytes: path.as_ref().as_bytes().to_owned(),
second_slash_index,
},
));
} }
if let Some(val) = self.inner.get_mut(subpath) { if let Some(val) = self.inner.get_mut(subpath) {
*val += 1; *val += 1;
break; break;
} }
self.inner.insert(subpath.to_owned(), 1); self.inner.insert(subpath.to_owned(), 1);
} }
Ok(()) Ok(())

rust/hg-core/src/lib.rs

fn from(e: std::io::Error) -> Self { fn from(e: std::io::Error) -> Self {
DirstatePackError::CorruptedEntry(e.to_string()) DirstatePackError::CorruptedEntry(e.to_string())
} }
} }
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub enum DirstateMapError { pub enum DirstateMapError {
PathNotFound(HgPathBuf), PathNotFound(HgPathBuf),
EmptyPath, EmptyPath,
ConsecutiveSlashes, InvalidPath(HgPathError),
} }
impl ToString for DirstateMapError { impl ToString for DirstateMapError {
fn to_string(&self) -> String { fn to_string(&self) -> String {
use crate::DirstateMapError::*;
match self { match self {
PathNotFound(_) => "expected a value, found none".to_string(), DirstateMapError::PathNotFound(_) => {
EmptyPath => "Overflow in dirstate.".to_string(), "expected a value, found none".to_string()
ConsecutiveSlashes => {
"found invalid consecutive slashes in path".to_string()
} }
DirstateMapError::EmptyPath => "Overflow in dirstate.".to_string(),
DirstateMapError::InvalidPath(e) => e.to_string(),
} }
} }
} }
pub enum DirstateError { pub enum DirstateError {
Parse(DirstateParseError), Parse(DirstateParseError),
Pack(DirstatePackError), Pack(DirstatePackError),
Map(DirstateMapError), Map(DirstateMapError),