Fixed couple of packed unaligned issues

[blietaer]

Hey !
Tried and fixed couple of packed unaligned issues.
New to Rust, so these are maybe not 'by the book', but since std:: is not available in such a bare metal approach, I worked around it...
...what do you think ? :)

Cheers,
Ben

[dwbrite]

Just a rando with no horse in this race - I don't see what you're solving here (except the new __nop). Maybe you forgot to commit some changes?

That said, pulling things out into shorter vars can be useful sometimes, but

• adding another variable into the mix is creating more information to keep in your head while reading,
• "brw" has no semantic meaning so I have to backtrack to figure out what it actually is anyway, and
• all of that compounds with the shadowing of brw in watchdog.rs, which will make future refactors much more difficult

and, if you do feel the need to pull things out into vars like this, please don't leave the old code in a comment. We have git for version control so we can see what it was before. The comments just add noise.

Hope I don't come off too harsh - keep at it :)

[blietaer]

Hey Devin, Many thanks for your feedback and your time: hehe not harsh, I guess that the name of the game of pull request and I am already lucky/happy you came back in my direction. Well, I guess you could consider this more like a request for support on your own code that does not compile out of the git-box anymore (well, at least with latest nightly...) Mmmh the __nop was already fixed, what I (also) got as an error is the many "packed alignment" errors (previously just warning, depending on your nightly, I guess). Hence the different vars I am adding as a workaround: sorry should I have provided more context here....not adding intermediate vars just for fun :P While your advice feedback are relevant and appreciated (from great benefit for me, still learning GitHUB and Rust), I would be eager to have then your fixes/patches in order to have the Teensy/post_1 example compiling out of the git-box once cloned, with latest nightly: any hint on this you could provide ? Cheers, Ben Le mar. 18 oct. 2022 à 05:41, Devin Brite ***@***.***> a écrit :

…

Just a rando with no horse in this race - I don't see what you're solving here (except the new __nop). Maybe you forgot to commit some changes? That said, pulling things out into shorter vars can be useful *sometimes*, but - adding another variable into the mix is creating more information to keep in your head while reading, - "brw" has no semantic meaning so I have to backtrack to figure out what it actually *is* - and all of that compounds with the shadowing of brw in watchdog.rs, which will make refactors much more difficult and, if you do feel the need to pull things out into vars like this, please don't leave the old code in a comment. We have git for version control so we can see what it was before. The comments just add noise. Hope I don't come off too harsh - keep at it :) — Reply to this email directly, view it on GitHub <#11 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJJFYCD6YXQLAQU44ORRNFDWDYL55ANCNFSM6AAAAAARHFYCKY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[dwbrite]

Suggested change

	let brw = self.pcr[p];
	// let mut pcr = core::ptr::read_volatile(&self.pcr[p]);
	let mut pcr = core::ptr::read_volatile(&brw);
	let mut pcr = core:ptr::read_volatile(&{brw});

We're using brw to Copy values before creating the reference, and the goal is just to align the value before use. Instead of creating a new variable for that copy, we can do this by placing the value in a block {...} which seems a little cleaner imo.

[dwbrite]

Not sure if/when we should be using raw pointers instead of copying

[blietaer]

Ah! Maybe cleaner than heavy copy indeed... :)

[dwbrite]

Now I see what you're doing - this is all in response to unaligned references becoming an error: rust-lang/rust#82523

@branan thoughts?

[blietaer]

Hehe yup, that's the point and I followed the very same link for the work around, although - as they suggested a std:: based solution (and we want/need to avoid std:: here in ARM bare metal) - I went for the ugly copy approach.
Let me check a core:: based approach here maybe.

[dwbrite]

The solution in that post is actually applicable to nostd environments using core, which is a (fairly large) subset of std.

core is the freestanding portion of std (all the parts that don't rely on anything provided by the operating system - e.g. memory allocation and file IO)

[blietaer]

Ah! OK, then let's revert my change to that approach, I can then probably cancel this P.R. and initiate another (in case we'd like to keep this nice tutorial up-to-date and/or compiling-friendly out-of-the-box (but not any horse in this race either...) ).