Skip to content

Commit

Permalink
Store NormalizeOpts inside NormalizationHandler
Browse files Browse the repository at this point in the history 
Instead of plumbing through individual flags to our internally used
NormalizationHandler, just pass in and store the complete NormalizeOpts
object. Doing so has the nice side-effect of making call sites perhaps a
tad more understandable, because we stop dealing with boolean flags
there.

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
@d-e-s-o
d-e-s-o committed Feb 26, 2025
1 parent fc9ae43 commit 3ff723b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 34 deletions.
32 changes: 13 additions & 19 deletions src/normalize/normalizer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ impl Normalizer {
Builder::default()
}

fn normalize_user_addrs_impl<A, E, M>(
&self,
fn normalize_user_addrs_impl<'slf, A, E, M>(
&'slf self,
addrs: A,
entries: E,
map_files: bool,
) -> Result<UserOutput<'_>>
opts: &NormalizeOpts,
) -> Result<UserOutput<'slf>>
where
A: ExactSizeIterator<Item = Addr> + Clone,
E: FnMut(Addr) -> Option<Result<M>>,
Expand All @@ -230,7 +230,7 @@ impl Normalizer {
&NoBuildIdReader as &dyn BuildIdReader
};

let mut handler = user::NormalizationHandler::new(reader, addrs_cnt, map_files);
let mut handler = user::NormalizationHandler::new(opts, reader, addrs_cnt);
let () = normalize_sorted_user_addrs_with_entries(addrs, entries, &mut handler)?;
debug_assert_eq!(handler.normalized.outputs.len(), addrs_cnt);
Ok(handler.normalized)
Expand All @@ -240,7 +240,7 @@ impl Normalizer {
&self,
addrs: A,
pid: Pid,
map_files: bool,
opts: &NormalizeOpts,
) -> Result<UserOutput>
where
A: ExactSizeIterator<Item = Addr> + Clone,
Expand All @@ -253,7 +253,7 @@ impl Normalizer {
if !self.cache_vmas {
let entries =
move |addr| query_procmap(&file, pid, addr, self.build_ids).transpose();
self.normalize_user_addrs_impl(addrs, entries, map_files)
self.normalize_user_addrs_impl(addrs, entries, opts)
} else {
let entries = self.cached_entries.get_or_try_insert(pid, || {
let mut entries = Vec::new();
Expand All @@ -269,13 +269,13 @@ impl Normalizer {

let mut entry_iter = entries.iter().map(Ok);
let entries = |_addr| entry_iter.next();
self.normalize_user_addrs_impl(addrs, entries, map_files)
self.normalize_user_addrs_impl(addrs, entries, opts)
}
} else {
if !self.cache_vmas {
let mut entry_iter = maps::parse_filtered(pid)?;
let entries = |_addr| entry_iter.next();
self.normalize_user_addrs_impl(addrs, entries, map_files)
self.normalize_user_addrs_impl(addrs, entries, opts)
} else {
let parsed = self.cached_entries.get_or_try_insert(pid, || {
// If we use the cached maps entries but don't have anything
Expand All @@ -287,7 +287,7 @@ impl Normalizer {

let mut entry_iter = parsed.iter().map(Ok);
let entries = |_addr| entry_iter.next();
self.normalize_user_addrs_impl(addrs, entries, map_files)
self.normalize_user_addrs_impl(addrs, entries, opts)
}
}
}
Expand Down Expand Up @@ -317,19 +317,13 @@ impl Normalizer {
addrs: &[Addr],
opts: &NormalizeOpts,
) -> Result<UserOutput> {
let NormalizeOpts {
sorted_addrs,
map_files,
_non_exhaustive: (),
} = *opts;

if sorted_addrs {
self.normalize_user_addrs_iter(addrs.iter().copied(), pid, map_files)
if opts.sorted_addrs {
self.normalize_user_addrs_iter(addrs.iter().copied(), pid, opts)
} else {
util::with_ordered_elems(
addrs,
|normalized: &mut UserOutput| normalized.outputs.as_mut_slice(),
|sorted_addrs| self.normalize_user_addrs_iter(sorted_addrs, pid, map_files),
|sorted_addrs| self.normalize_user_addrs_iter(sorted_addrs, pid, opts),
)
}
}
Expand Down
31 changes: 16 additions & 15 deletions src/normalize/user.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use super::meta::Elf;
use super::meta::Unknown;
use super::meta::UserMeta;
use super::normalizer::Output;
use super::NormalizeOpts;
use super::Reason;


Expand Down Expand Up @@ -116,37 +117,37 @@ pub(crate) trait Handler<D = ()> {
}


pub(super) struct NormalizationHandler<'reader, 'src> {
pub(super) struct NormalizationHandler<'call, 'src> {
/// The user output we are building up.
pub normalized: UserOutput<'src>,
/// The options used as part of the normalization request.
normalize_opts: &'call NormalizeOpts,
/// The build ID reader to use.
build_id_reader: &'reader dyn BuildIdReader<'src>,
build_id_reader: &'call dyn BuildIdReader<'src>,
/// Lookup table from path (as used in each proc maps entry) to index into
/// `output.meta`.
meta_lookup: HashMap<PathBuf, usize>,
/// A mapping from [`Reason`] to the index of the `Unknown` entry with this
/// very reason in `meta_lookup`, if any.
unknown_cache: HashMap<Reason, usize>,
/// Report `map_files` entries instead of symbolic paths.
map_files: bool,
}

impl<'reader, 'src> NormalizationHandler<'reader, 'src> {
impl<'call, 'src> NormalizationHandler<'call, 'src> {
/// Instantiate a new `NormalizationHandler` object.
pub(crate) fn new(
reader: &'reader dyn BuildIdReader<'src>,
opts: &'call NormalizeOpts,
reader: &'call dyn BuildIdReader<'src>,
addr_cnt: usize,
map_files: bool,
) -> Self {
Self {
normalized: UserOutput {
outputs: Vec::with_capacity(addr_cnt),
meta: Vec::new(),
},
normalize_opts: opts,
build_id_reader: reader,
meta_lookup: HashMap::new(),
unknown_cache: HashMap::new(),
map_files,
}
}
}
Expand All @@ -162,7 +163,7 @@ impl Handler<Reason> for NormalizationHandler<'_, '_> {
fn handle_entry_addr(&mut self, addr: Addr, entry: &MapsEntry) -> Result<()> {
match &entry.path_name {
Some(PathName::Path(entry_path)) => {
let path = if self.map_files {
let path = if self.normalize_opts.map_files {
&entry_path.maps_file
} else {
&entry_path.symbolic_path
Expand Down Expand Up @@ -321,14 +322,14 @@ mod tests {

let pid = Pid::Slf;
let addrs = [unknown_addr as Addr];
let map_files = false;

let mut entry_iter = maps::parse_file(maps.as_bytes(), pid)
.filter(|result| result.as_ref().map(maps::filter_relevant).unwrap_or(true));
let entries = |_addr| entry_iter.next();

let opts = NormalizeOpts::default();
let reader = NoBuildIdReader;
let mut handler = NormalizationHandler::new(&reader, addrs.len(), map_files);
let mut handler = NormalizationHandler::new(&opts, &reader, addrs.len());
let () = normalize_sorted_user_addrs_with_entries(
addrs.as_slice().iter().copied(),
entries,
Expand Down Expand Up @@ -359,7 +360,6 @@ mod tests {
#[test]
fn normalize_various_entries() {
let addrs = [0x10000, 0x30000];
let map_files = false;

let mut entry_iter = [
Ok(MapsEntry {
Expand All @@ -382,8 +382,9 @@ mod tests {
.into_iter();
let entries = |_addr| entry_iter.next();

let opts = NormalizeOpts::default();
let reader = NoBuildIdReader;
let mut handler = NormalizationHandler::new(&reader, addrs.len(), map_files);
let mut handler = NormalizationHandler::new(&opts, &reader, addrs.len());
let () = normalize_sorted_user_addrs_with_entries(
addrs.as_slice().iter().copied(),
entries,
Expand Down Expand Up @@ -414,13 +415,13 @@ mod tests {
#[test]
fn build_id_read_failures() {
let addrs = [build_id_read_failures as Addr];
let map_files = false;

let mut entry_iter = maps::parse_filtered(Pid::Slf).unwrap();
let entries = |_addr| entry_iter.next();

let opts = NormalizeOpts::default();
let reader = FailingBuildIdReader;
let mut handler = NormalizationHandler::new(&reader, addrs.len(), map_files);
let mut handler = NormalizationHandler::new(&opts, &reader, addrs.len());
let () = normalize_sorted_user_addrs_with_entries(
addrs.as_slice().iter().copied(),
entries,
Expand Down

0 comments on commit 3ff723b

Please sign in to comment.