rust-cli / rust-cli/anstyle

Unnecessary unsafe in anstyle-parse's osc_dispatch?

Open
#300 4 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
HTML
Stars
171
Forks
44
Avg merge
2h 34m
Merged PRs (30d)
6

Description

anstyle-parse has:

    #[inline]
    fn osc_dispatch<P: Perform>(&self, performer: &mut P, byte: u8) {
        let mut slices: [MaybeUninit<&[u8]>; MAX_OSC_PARAMS] =
            unsafe { MaybeUninit::uninit().assume_init() };

        for (i, slice) in slices.iter_mut().enumerate().take(self.osc_num_params) {
            let indices = self.osc_params[i];
            *slice = MaybeUninit::new(&self.osc_raw[indices.0..indices.1]);
        }

        unsafe {
            let num_params = self.osc_num_params;
            let params = &slices[..num_params] as *const [MaybeUninit<&[u8]>] as *const [&[u8]];
            performer.osc_dispatch(&*params, byte == 0x07);
        }
    }

this caught my eye because I wasn't sure if the initialisation of slices is sound, but after some thought it's not clear why MaybeUninit is needed at all - I don't really understand it, but is there any reason why you can't instead create an array of slices that is initialised using zero-length slices, and then overwrite them where possible? I.e.

    #[inline]
    fn osc_dispatch<P: Perform>(&self, performer: &mut P, byte: u8) {
        let mut slices: [&[u8]; MAX_OSC_PARAMS] = [&[]; MAX_OSC_PARAMS];

        for (i, slice) in slices.iter_mut().enumerate().take(self.osc_num_params) {
            let indices = self.osc_params[i];
            *slice = &self.osc_raw[indices.0..indices.1];
        }

        performer.osc_dispatch(&slices[..self.osc_num_params], byte == 0x07);
    }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate the osc_dispatch implementation in anstyle-parse and inspect how osc_params, osc_raw, and MAX_OSC_PARAMS constrain the slice construction. Compare the MaybeUninit approach with zero-length slice initialization, then run the existing anstyle-parse tests and confirm the OSC dispatch behavior remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.