# Memcpy is backwards

**URL:** <https://internals.rust-lang.org/t/memcpy-is-backwards/1797>\
**Category:** Uncategorized\
**Created:** [April 2, 2015, 3:59pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797 "2015-04-02T15:59:46Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![cgaebel](https://avatars.discourse-cdn.com/v4/letter/c/da6949/32.png) [@cgaebel](https://internals.rust-lang.org/u/cgaebel)\
**Post date:** [April 2, 2015, 3:59pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/1 "2015-04-02T15:59:46Z")

</div>

> <https://github.com/rust-lang/rust/pull/23866>

This commit swapped the parameter order or `src` and `dest` to rust's memcpy and memmove equivalents: `ptr::copy` and `ptr::copy_nonoverlapping`. This was a subtle change to a `#[stable]` API, without an RFC. Can it be reverted?

For reference, code like this is now subtly broken:

```rust
let dst: *mut u8 = ...;
let src: *mut u8 = ...;
ptr::copy_nonoverlapping(dst, src, 16);

```

---

<div class="post-metadata">

**Author:** ![bfops](https://avatars.discourse-cdn.com/v4/letter/b/50afbb/32.png) [@bfops](https://internals.rust-lang.org/u/bfops)\
**Post date:** [April 2, 2015, 4:09pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/2 "2015-04-02T16:09:04Z")

</div>

It definitely seems a little scary to allow that behaviour to change completely silently. If it’s marked unstable, then users would at least get a warning when they don’t get an error from the `const`/`mut` swap.

---

<div class="post-metadata">

**Author:** ![tbu](https://avatars.discourse-cdn.com/v4/letter/t/839c29/32.png) [@tbu](https://internals.rust-lang.org/u/tbu)\
**Post date:** [April 2, 2015, 7:11pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/3 "2015-04-02T19:11:06Z")

</div>

Thanks for posting this!

---

<div class="post-metadata">

**Author:** ![lambdaburrito](https://avatars.discourse-cdn.com/v4/letter/l/e47c2d/32.png) [@lambdaburrito](https://internals.rust-lang.org/u/lambdaburrito)\
**Post date:** [April 2, 2015, 11:12pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/4 "2015-04-02T23:12:58Z")

</div>

A lot of libraries are silently broken because of this.

---

<div class="post-metadata">

**Author:** ![lambdaburrito](https://avatars.discourse-cdn.com/v4/letter/l/e47c2d/32.png) [@lambdaburrito](https://internals.rust-lang.org/u/lambdaburrito)\
**Post date:** [April 2, 2015, 11:18pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/5 "2015-04-02T23:18:51Z")

</div>

Also I think there should be unit tests to ensure this doesn’t happen again. I’m happy to contribute tests as I’m soon to put Rust code in production and this completely broke our system.

---

<div class="post-metadata">

**Author:** ![seanmonstar](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/seanmonstar/32/36_2.png) [@seanmonstar](https://internals.rust-lang.org/u/seanmonstar)\
**Post date:** [April 2, 2015, 11:55pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/6 "2015-04-02T23:55:26Z")

</div>

Yes, this hurt and was surprising. But unit tests caught it (for me). Make sure you crates have tests, and you shouldn’t be bitten badly.

---

<div class="post-metadata">

**Author:** ![bfops](https://avatars.discourse-cdn.com/v4/letter/b/50afbb/32.png) [@bfops](https://internals.rust-lang.org/u/bfops)\
**Post date:** [April 3, 2015, 12:04am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/7 "2015-04-03T00:04:01Z")

</div>

Tests can’t really be reasonably expected to cover every possibility for “my library’s behavior changed silently under me”, and even so, it’s way harder to backtrace failing tests to silently changing behavior than it is to read a warning.

---

<div class="post-metadata">

**Author:** ![mitsuhiko](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mitsuhiko/32/379_2.png) [@mitsuhiko](https://internals.rust-lang.org/u/mitsuhiko)\
**Post date:** [April 4, 2015, 1:14pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/8 "2015-04-04T13:14:19Z")

</div>

I got bitten by this as well. More so: I think the new argument order is just wrong. This code lives in unsafe blocks where you call lots of C APIs which all follow the dst, src argument order.

---

<div class="post-metadata">

**Author:** ![cgaebel](https://avatars.discourse-cdn.com/v4/letter/c/da6949/32.png) [@cgaebel](https://internals.rust-lang.org/u/cgaebel)\
**Post date:** [April 4, 2015, 1:51pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/9 "2015-04-04T13:51:29Z")

</div>

Ah. I see it wasn’t clear that I’m against this new memcpy argument order. I am. Y’all would’ve known that if there was an RFC.

---

<div class="post-metadata">

**Author:** ![bstrie](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/bstrie/32/94_2.png) [@bstrie](https://internals.rust-lang.org/u/bstrie)\
**Post date:** [April 7, 2015, 2:07am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/10 "2015-04-07T02:07:17Z")

</div>

I agree that an RFC was warranted here. Recall that “community members get angry and lose faith in the devs when we pull the rug out from under them with surprising changes” was literally the impetus for the RFC process in the first place.

---

<div class="post-metadata">

**Author:** ![tikue](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/tikue/32/4053_2.png) [@tikue](https://internals.rust-lang.org/u/tikue)\
**Post date:** [April 7, 2015, 2:34am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/11 "2015-04-07T02:34:58Z")

</div>

Ouch. This hurts. Yeah, I think a revert (until an RFC is approved) might be warranted as a sign of good faith. Though perhaps that’d just break things even further at this point…

---

<div class="post-metadata">

**Author:** ![pcwalton](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pcwalton/32/207_2.png) [@pcwalton](https://internals.rust-lang.org/u/pcwalton)\
**Post date:** [April 7, 2015, 6:44am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/12 "2015-04-07T06:44:31Z")

</div>

@bstrie, please don’t try to turn this into a core-team-versus-community thing. I’m on the core team and had no idea this was happening, and I think this was the wrong call.

---

<div class="post-metadata">

**Author:** ![bstrie](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/bstrie/32/94_2.png) [@bstrie](https://internals.rust-lang.org/u/bstrie)\
**Post date:** [April 7, 2015, 7:38am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/13 "2015-04-07T07:38:11Z")

</div>

Not trying to cast stones or rally a mob here, just discomfited at this sense of deja vu. I’m sure that everyone’s intentions here were noble but people need assurance that contentious changes will be done transparently.

---

<div class="post-metadata">

**Author:** ![CloudiDust](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cloudidust/32/197_2.png) [@CloudiDust](https://internals.rust-lang.org/u/CloudiDust)\
**Post date:** [April 7, 2015, 8:39am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/14 "2015-04-07T08:39:11Z")

</div>

One problem with reverting is that, the change is already out in the _beta_ (not a “normal” nighty), if we revert, there can be more confusion. (Which revision expects which order?)

One solution may be, instead of reverting, we _rename_ the low level functions to somehow clearly indicate the `input -> output` order, so people will not expect the C order.

I’ll prepare an RFC for this and include both candidates.

---

<div class="post-metadata">

**Author:** ![Manishearth](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/manishearth/32/50_2.png) [@Manishearth](https://internals.rust-lang.org/u/Manishearth)\
**Post date:** [April 7, 2015, 8:45am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/15 "2015-04-07T08:45:50Z")

</div>

For calls like this which can easily cause issues if gotten wrong, perhaps we should newtype the parametes? `memcpy(From(x), To(y))` or whatever.

---

<div class="post-metadata">

**Author:** ![Manishearth](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/manishearth/32/50_2.png) [@Manishearth](https://internals.rust-lang.org/u/Manishearth)\
**Post date:** [April 7, 2015, 9:17am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/16 "2015-04-07T09:17:59Z")

</div>

Also, filed

> <https://github.com/rust-lang/rfcs/issues/1041>
>
> We should decide on a set of changes that should always need an RfC. I haven't come up with an exhaustive...

---

<div class="post-metadata">

**Author:** ![bluss](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/bluss/32/2264_2.png) [@bluss](https://internals.rust-lang.org/u/bluss)\
**Post date:** [April 7, 2015, 10:50am UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/17 "2015-04-07T10:50:36Z")

</div>

It’s happed now, the argument order is a bikeshed issue, the only problem is the breakage.

If we want to do the best job possible, most realistic mitigation in my mind would maybe be to add a lint that warns when `*mut` is coerced to `*const`, possibly even targeting the lint at specific functions.

---

<div class="post-metadata">

**Author:** ![CloudiDust](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cloudidust/32/197_2.png) [@CloudiDust](https://internals.rust-lang.org/u/CloudiDust)\
**Post date:** [April 7, 2015, 12:54pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/18 "2015-04-07T12:54:39Z")

</div>

@Manishearth, this seems a bit heavy weight. A lint as suggested by @blake may be better.

Renaming is not as good a solution as I initially thought. If we add the ordering information to those functions’ names, then why don’t other `copy` functions like `std::io::copy` have ordering information too? Is the lack of such information indicating that arguments are reversed for `std::io::copy`? (The answer is no, but this is an understandable interpretation.)

Also, the functions’ current names are not that like `memcpy` and `memmove` after all. (YMMV of course.) As long as people _are_ aware (or made aware) that Rust _consistently_ follow the `src, dst` order, it can be okay.

---

<div class="post-metadata">

**Author:** ![mkpankov](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/mkpankov/32/7100_2.png) [@mkpankov](https://internals.rust-lang.org/u/mkpankov)\
**Post date:** [April 7, 2015, 3:27pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/19 "2015-04-07T15:27:55Z")

</div>

I wonder why was this change required in the first place.

---

<div class="post-metadata">

**Author:** ![nikomatsakis](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/nikomatsakis/32/5410_2.png) [@nikomatsakis](https://internals.rust-lang.org/u/nikomatsakis)\
**Post date:** [April 7, 2015, 3:35pm UTC](https://internals.rust-lang.org/t/memcpy-is-backwards/1797/20 "2015-04-07T15:35:55Z")

</div>

Hmm. My two cents:

Clearly, this change ought to at minimum have been advertised more widely, whether or not an RFC was required. I wasn’t involved in the decision, but I can easily imagine though that it seemed harmless enough (“harmonize the ordering of arguments across functions!”). Mistakes happen. The good part is that this is a one-time sort of problem, I think. We won’t even consider silent breaking changes like this after 1.0, clearly.

What’s not entirely clear to me is what is the best thing to do _now_. After all, the new ordering _is_ more consistent, and beta has shipped—introducing yet another breaking change doesn’t necessarily feel like the best solution. A targeted lint might be a good compromise, to help people get their code fixed. Or maybe we just ride this out—by the time a lint is ready, perhaps most crates will already be working again.

I guess the key question is whether people disagree with the _substance_ of the decision (i.e., internal consistency is less important than consistency with `memcpy`, or perhaps that I/O APIs ought to have been changed instead) or just the silent, breaking nature of it. I personally tend to think the change was right on the merits—consistency is better than not, and I don’t care as to which ordering we use—but it ought to have been (at minimum) advertised more widely. Still, I’m somewhat on the fence.

**UPDATE:** tweaked wording, formatting

[Next page](https://internals.rust-lang.org/t/memcpy-is-backwards/1797.md?page=2)
