CODEWITHSUNDEEP
rust
Gatonito
Gatonito

Reputation: 2164

Is this mem::transmute::<&str, &'static str>(k) safe?

I'm looking at this code which is a very simple library with just one file and mostly tests, so it's short. There's a struct that I'm trying to understand:

pub struct ChallengeFields(HashMap<UniCase<CowStr>, (String, Quote)>);

struct CowStr(Cow<'static, str>);

There's a line where it does

pub fn get(&self, k: &str) -> Option<&String> {
    self.0
        .get(&UniCase(CowStr(Cow::Borrowed(unsafe {
            mem::transmute::<&str, &'static str>(k)
        }))))
        .map(|&(ref s, _)| s)
}

I'm annoyed by this unsafe operation. I think CowStr is a Cow with 'static lifetime otherwise it'd be hard or impossible to store strs inside the map. Because of that, when I try to get something inside this map, the str in question has to have 'static lifetime. This is the reason for the transmute, right? If so, why simply not use String, so we can get rid of lifetimes and thus transmute? I don't like unsafe, and reading about transmute it looks very unsafe.

Also, I don't see why Cow is needed at all.

Upvotes: 3

Views: 466

Answers (1)

Masklinn
Masklinn

Reputation: 42227

I think CowStr is a Cow with 'static lifetime otherwise it'd be hard or impossible to store strs inside the map.

Well yes and no, you can store &'static str inside a hashmap with no issue, the problem is that you can't store both &'static str and String.

Am I rigth? If so, why simply not use String, so we can get rid of lifetimes and thus transmute?

I assume that is an optimisation: with String you'd have to create an allocation every time you want to insert a challenge in the map, but if the overwhelming majority of challenge names would be Digest and Basic then that's a waste of time (and memory but mostly time), but at the same time you'd have to support String for custom auth schemes.

Now maybe in the grand scheme of things this is not an optimisation which actually matter and it'd be better off not doing that, I couldn't tell you.

I don't like unsafe, and reading about transmute it looks very unsafe.

It's a debatable thing to do, but in this case it's "safe", in the sense that the reference is valid for the entirety of the HashMap::get call and we know that that call doesn't keep the reference alive (it's reliance on an implementation detail which is a bit risky, but the odds that that would change are basically nil as it wouldn't make much sense).

Extending lifetimes is not in-and-of-itself UB (the mem::transmute documentation literally provides an example doing that), but requires care as you must avoid it causing UBs (the most likely being a dangling reference).

Upvotes: 3

Related Questions