mirror of
https://github.com/nkbai/book.git
synced 2026-09-04 05:32:47 +08:00
Merge #90
90: Move "last_state = state" assignment r=adamgreig a=adamgreen
I believe that I have found a bug in the samples found in the concurrency chapter where the value of the `last_state` variable should be updated on each loop iteration but it is only updated on the detection of the first leading edge. For example:
```rust
loop {
let state = read_signal_level();
if state && !last_state {
last_state = state;
// DANGER - Not actually safe! Could cause data races.
unsafe { COUNTER += 1 };
}
}
```
I think that loop should actually be written as:
```rust
loop {
let state = read_signal_level();
if state && !last_state {
// DANGER - Not actually safe! Could cause data races.
unsafe { COUNTER += 1 };
}
last_state = state;
}
```
As the code exists today, I think it would detect the first leading edge, set last_state to true and then never be updated again which would also mean that it never detects another leading edge.
Co-authored-by: Adam Green <adamgreen@users.noreply.github.com>
This commit is contained in:
@@ -68,10 +68,10 @@ fn main() -> ! {
|
||||
loop {
|
||||
let state = read_signal_level();
|
||||
if state && !last_state {
|
||||
last_state = state;
|
||||
// DANGER - Not actually safe! Could cause data races.
|
||||
unsafe { COUNTER += 1 };
|
||||
}
|
||||
last_state = state;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,12 +109,12 @@ fn main() -> ! {
|
||||
loop {
|
||||
let state = read_signal_level();
|
||||
if state && !last_state {
|
||||
last_state = state;
|
||||
// New critical section ensures synchronised access to COUNTER
|
||||
cortex_m::interrupt::free(|_| {
|
||||
unsafe { COUNTER += 1 };
|
||||
});
|
||||
}
|
||||
last_state = state;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -172,10 +172,10 @@ fn main() -> ! {
|
||||
loop {
|
||||
let state = read_signal_level();
|
||||
if state && !last_state {
|
||||
last_state = state;
|
||||
// Use `fetch_add` to atomically add 1 to COUNTER
|
||||
COUNTER.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
last_state = state;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -254,10 +254,10 @@ fn main() -> ! {
|
||||
loop {
|
||||
let state = read_signal_level();
|
||||
if state && !last_state {
|
||||
last_state = state;
|
||||
// No unsafe here!
|
||||
interrupt::free(|cs| COUNTER.increment(cs));
|
||||
}
|
||||
last_state = state;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -353,10 +353,10 @@ fn main() -> ! {
|
||||
loop {
|
||||
let state = read_signal_level();
|
||||
if state && !last_state {
|
||||
last_state = state;
|
||||
interrupt::free(|cs|
|
||||
COUNTER.borrow(cs).set(COUNTER.borrow(cs).get() + 1));
|
||||
}
|
||||
last_state = state;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -448,14 +448,13 @@ fn main() -> ! {
|
||||
});
|
||||
|
||||
if state && !last_state {
|
||||
last_state = state;
|
||||
|
||||
// Set PA1 high if we've seen a rising edge on PA0.
|
||||
interrupt::free(|cs| {
|
||||
let gpioa = MY_GPIO.borrow(cs).borrow();
|
||||
gpioa.as_ref().unwrap().odr.modify(|_, w| w.odr1().set_bit());
|
||||
});
|
||||
}
|
||||
last_state = state;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user