How to write a simple driver in bare metal systems: volatile, memory barrier, critical sections and so on

On 10/25/2021 21:33, Niklas Holsti wrote:
> ....
> 
> If the FIFO implementation is based on just two pointers (read and 
> write), and each pointer is modified by just one of the two threads 
> (main thread = reader, and interrupt handler = writer), and those 
> modifications are both "volatile" AND atomic (which has not been 
> discussed so far, IIRC...), then one can do without a critical region. 
> But then detection of a full buffer needs one "wasted" element in the 
> buffer.

Why atomic? No need for that unless more than one interrupted task would
want to read from the fifo at the same time, which is nonsense. [I once
wasted a day looking at a garbled input from an auxiliary HV to a netMCA
only to discover I had left a shell to start via the same UART during
boot, through an outdated (but available) driver accessing the same
UART the standard driver through which the HV was used....
This across half the planet, customer was in South Africa. Not
an experience anybody would ask for, I can tell you :)].
Just like there is no need to mask interrupts, as you mentioned I
had said before.

> To avoid the wasted element, one could add a "full"/"not full" Boolean 
> flag. But that flag would be modified by both threads, and should be 
> modified atomically together with the pointer modifications, which (I 
> think) means that a critical section is needed.

Now this is where atomic access is necessary - for no good reason in
this case, as mentioned before, but if one wants to bang their head
in the wall this is the proper way to do it.
As for "volatile" I can't say much, but if this is the way to make
the compiler access every time the address declared such instead of
using some stale data it has then it would be needed of
course.

On 2021-10-25 22:09, Dimiter_Popoff wrote:
> On 10/25/2021 21:33, Niklas Holsti wrote:
>> ....
>>
>> If the FIFO implementation is based on just two pointers (read and 
>> write), and each pointer is modified by just one of the two threads 
>> (main thread = reader, and interrupt handler = writer), and those 
>> modifications are both "volatile" AND atomic (which has not been 
>> discussed so far, IIRC...), then one can do without a critical region. 
>> But then detection of a full buffer needs one "wasted" element in the 
>> buffer.
> 
> Why atomic?


If the read/write pointers/indices are, say, 16 bits, but the processor 
has only 8-bit store/load instructions, updating a pointer/index happens 
non-atomically, 8 bits at a time, and the interrupt handler can read a 
half-updated value if the interrupt happens in the middle of an update. 
That would certainly mess up the comparison between the read and write 
points in the interrupt handler.

In the OP's code, I suppose (but I don't recall) that the indices are 8 
bits, so probably atomically readable and writable.

On 10/25/2021 22:53, Niklas Holsti wrote:
> On 2021-10-25 22:09, Dimiter_Popoff wrote:
>> On 10/25/2021 21:33, Niklas Holsti wrote:
>>> ....
>>>
>>> If the FIFO implementation is based on just two pointers (read and 
>>> write), and each pointer is modified by just one of the two threads 
>>> (main thread = reader, and interrupt handler = writer), and those 
>>> modifications are both "volatile" AND atomic (which has not been 
>>> discussed so far, IIRC...), then one can do without a critical 
>>> region. But then detection of a full buffer needs one "wasted" 
>>> element in the buffer.
>>
>> Why atomic?
> 
> 
> If the read/write pointers/indices are, say, 16 bits, but the processor 
> has only 8-bit store/load instructions, updating a pointer/index happens 
> non-atomically, 8 bits at a time, and the interrupt handler can read a 
> half-updated value if the interrupt happens in the middle of an update. 
> That would certainly mess up the comparison between the read and write 
> points in the interrupt handler.
> 
> In the OP's code, I suppose (but I don't recall) that the indices are 8 
> bits, so probably atomically readable and writable.
> 

Ah, well, this is a possible scenario in a multicore system (or single
core if the two bytes are written by separate opcodes).

Don Y <blockedofcourse@foo.invalid> wrote:
> On 10/24/2021 4:14 AM, Dimiter_Popoff wrote:
> >> Disable interrupts while accessing the fifo. you really have to.
> >> alternatively you'll often get away not using a fifo at all,
> >> unless you're blocking for a long while in some part of the code.
> > 
> > Why would you do that. The fifo write pointer is only modified by
> > the interrupt handler, the read pointer is only modified by the
> > interrupted code. Has been done so for times immemorial.
> 
> The OPs code doesn't differentiate between FIFO full and empty.

If you read carefuly what he wrote you would know that he does.
The trick he uses is that his indices may point outside buffer:
empty is equal indices, full is difference equal to buffer
size.  Of course his approach has its own limitations, like
buffer size being power of 2 and with 8 bit indices maximal
buffer size is 128.

-- 
                              Waldek Hebisch

On 25/10/21 7:09 pm, Niklas Holsti wrote:
> On 2021-10-24 23:27, Dimiter_Popoff wrote:
>> On 10/24/2021 22:54, Don Y wrote:
>>> On 10/24/2021 4:14 AM, Dimiter_Popoff wrote:
>>>>> Disable interrupts while accessing the fifo. you really have to.
>>>>> alternatively you'll often get away not using a fifo at all,
>>>>> unless you're blocking for a long while in some part of the code.
>>>>
>>>> Why would you do that. The fifo write pointer is only modified by
>>>> the interrupt handler, the read pointer is only modified by the
>>>> interrupted code. Has been done so for times immemorial.
>>>
>>> The OPs code doesn't differentiate between FIFO full and empty.
>>
>> So he should fix that first, there is no sane reason why not.
>> Few things are simpler to do than that.
> 
> 
>  �� [snip]
>> Whatever handshakes he makes there is no problem knowing whether
>> the fifo is full - just check if the position the write pointer
>> will have after putting the next byte matches the read pointer
>> at the moment.� Like I said before, few things are simpler than
>> that, can't imagine someone working as a programmer being
>> stuck at *that*.
> 
> That simple check would require keeping a maximum of only N-1 entries in 
> the N-position FIFO buffer, and the OP explicitly said they did not want 
> to allocate an unused place in the buffer (which I think is unreasonable 
> of the OP, but that is only IMO).

In my opinion too. If you're going to waste a memory cell, why not use 
it for a count variable instead of an unused element?

CH

Il 25/10/2021 20:33, Niklas Holsti ha scritto:
> On 2021-10-25 20:52, pozz wrote:
>> Il 25/10/2021 17:34, Niklas Holsti ha scritto:
>>> On 2021-10-25 16:04, Dimiter_Popoff wrote:
>>>> On 10/25/2021 11:09, Niklas Holsti wrote:
>>>>> On 2021-10-24 23:27, Dimiter_Popoff wrote:
>>>>>> On 10/24/2021 22:54, Don Y wrote:
>>>>>>> On 10/24/2021 4:14 AM, Dimiter_Popoff wrote:
>>>>>>>>> Disable interrupts while accessing the fifo. you really have to.
>>>>>>>>> alternatively you'll often get away not using a fifo at all,
>>>>>>>>> unless you're blocking for a long while in some part of the code.
>>>>>>>>
>>>>>>>> Why would you do that. The fifo write pointer is only modified by
>>>>>>>> the interrupt handler, the read pointer is only modified by the
>>>>>>>> interrupted code. Has been done so for times immemorial.
>>>>>>>
>>>>>>> The OPs code doesn't differentiate between FIFO full and empty.
> 
> 
> (I suspect something is not quite right with the attributions of the 
> quotations above -- Dimiter probably did not suggest disabling 
> interrupts -- but no matter.)
> 
>  �� [snip]
> 
> 
>> When I have a small (<256) power-of-two (16, 32, 64, 128) buffer (and 
>> this is the case for a UART receiving ring-buffer), I like to use this 
>> implementation that works and doesn't waste any element.
>>
>> However I know this isn't the best implementation ever and it's a pity 
>> the thread emphasis has been against this implementation (that was 
>> used as *one* implementation just to have an example to discuss on).
>>
>> The main point was the use of volatile (and other techniques) to 
>> guarantee a correct compiler output, whatever legal (respect the C 
>> standard) optimizations the compiler thinks to do.
>>
>> It seems to me the arguments againts or for volatile are completely 
>> indipendent from the implementation of ring-buffer.
> 
> 
> Of course "volatile" is needed, in general, whenever anything is written 
> in one thread and read in another. The issue, I think, is when 
> "volatile" is _enough_.
> 
> I feel that detection of a full buffer (FIFO overflow) is required for a 
> proper ring buffer implementation, and that has implications for the 
> data structure needed, and that has implications for whether critical 
> sections are needed.
> 
> If the FIFO implementation is based on just two pointers (read and 
> write), and each pointer is modified by just one of the two threads 
> (main thread = reader, and interrupt handler = writer), and those 
> modifications are both "volatile" AND atomic (which has not been 
> discussed so far, IIRC...), then one can do without a critical region. 
> But then detection of a full buffer needs one "wasted" element in the 
> buffer.

Yeah, this is exactly the topic of my original post. Anyway it seems 
what you say isn't always correct. As per C standard, the compiler could 
reorder instructions that involve non-volatile data. So, even in your 
simplified scenario (atomic access for indexes), volatile for the head 
only (that ISR changes) is not sufficient.

The function called in the mainloop and that get data from the buffer 
access three variables: head (changed in ISR), tail (not changed in ISR) 
and buf[] (written in ISR ad read in mainloop).

The get function firstly check if some data is available in the FIFO and 
*next* read from buf[]. However compiler could rearrange instructions so 
reading from buf[] at first and then checking FIFO empty condition.
If the compiler goes this way, errors could occur during execution.

My original question was exactly if this could happen (without breaking 
C specifications) and, if yes, how to avoid this: volatile? critical 
section? memory barrier?

David Brown said this is possible and suggested to access both head and 
buf[] as volatile in get() function, forcing the compiler to respect the 
order of instructions.


> To avoid the wasted element, one could add a "full"/"not full" Boolean 
> flag. But that flag would be modified by both threads, and should be 
> modified atomically together with the pointer modifications, which (I 
> think) means that a critical section is needed.

On 10/25/21 2:15 PM, pozz wrote:
> Il 23/10/2021 18:09, David Brown ha scritto:
> [...]
>> Marking "in" and "buf" as volatile is /far/ better than using a critical
>> section, and likely to be more efficient than a memory barrier.  You can
>> also use volatileAccess rather than making buf volatile, and it is often
>> slightly more efficient to cache volatile variables in a local variable
>> while working with them.
> 
> I think I got your point, but I'm wondering why there are plenty of 
> examples of ring-buffer implementations that don't use volatile at all, 
> even if the author explicitly refers to interrupts and multithreading.
> 
> Just an example[1] by Quantum Leaps. It promises to be a *lock-free* (I 
> think thread-safe) ring-buffer implementation in the scenario of single 
> producer/single consumer (that is my scenario too).
> 
> In the source code there's no use of volatile. I could call 
> RingBuf_put() in my rx uart ISR and call RingBuf_get() in my mainloop code.
> 
>  From what I learned from you, this code usually works, but the standard 
> doesn't guarantee it will work with every old, current and future 
> compilers.
> 
> 
> 
> [1] https://github.com/QuantumLeaps/lock-free-ring-buffer

The issue with not using 'volatile' (or some similar memory barrier) is 
that without it, the implementation is allowed to delay the actual write 
of the results into the variable.

If optimization is limited to just within a single translation unit, you 
can force it to work by having the execution path leave the translation 
unit, but with whole program optimization, it is theoretically possible 
that the implementation sees that the thread of execution NEVER needs it 
to be spilled out of the registers to memory, so the ISR will never see 
the change.

Don Y <blockedofcourse@foo.invalid> wrote:
> On 10/25/2021 2:32 PM, antispam@math.uni.wroc.pl wrote:
> > Don Y <blockedofcourse@foo.invalid> wrote:
> >> On 10/24/2021 4:14 AM, Dimiter_Popoff wrote:
> >>>> Disable interrupts while accessing the fifo. you really have to.
> >>>> alternatively you'll often get away not using a fifo at all,
> >>>> unless you're blocking for a long while in some part of the code.
> >>>
> >>> Why would you do that. The fifo write pointer is only modified by
> >>> the interrupt handler, the read pointer is only modified by the
> >>> interrupted code. Has been done so for times immemorial.
> >>
> >> The OPs code doesn't differentiate between FIFO full and empty.
> > 
> > If you read carefuly what he wrote you would know that he does.
> > The trick he uses is that his indices may point outside buffer:
> > empty is equal indices, full is difference equal to buffer
> 
> Doesn't matter as any index can increase by any amount and
> invalidate the "reality" of the buffer's contents (i.e.
> actual number of characters that have been tranfered to
> that region of memory).

AFAIK OP considers this not a problem in his application.
Of course, if such changes were a problem he would need to
add test preventing writing to full buffer (he already have
test preventing reading from empty buffer).

> Buffer size is 128, for example.  in is 127, out is 127.
> What's that mean?

Empty buffer.

>  Can you tell me what has happened prior
> to this point in time?  Have 127 characters been received?
> Or, 383?  Or, 1151?

Does not matter.

> How many characters have been removed from the buffer?
> (same numeric examples).

The same as has been stored.  Point is that received is
always bigger or equal to removed and does not exceed
removed by more than 128.  So you can exactly recover
difference between received and removed.

> The biggest practical limitation is that of expectations of
> other developers who may inherit (or copy) his code expecting
> the FIFO to be "well behaved".

Well, personally I would avoid storing to full buffer.  And
even on small MCU it is not clear for me if his "savings"
are worth it.  But his core design is sound.

Concerning other developers, I always working on assumption
that code is "as is" and any claim what it is doing are of
limited value unless there is convincing argument (proof
or outline of proof) what it is doing.  Fact that code
worked well in past system(s) is rather unconvincing.
I have seen small (few lines) pieces of code that contained
multiple bugs.  And that code was in "production" use
for several years and passed its tests.

Certainly code like FIFO-s where there are multiple tradeofs
and actual code tends to be relatively small deserves
examination before re-use.

-- 
                              Waldek Hebisch

On 10/26/2021 5:20 PM, antispam@math.uni.wroc.pl wrote:
> Don Y <blockedofcourse@foo.invalid> wrote:
>> On 10/25/2021 2:32 PM, antispam@math.uni.wroc.pl wrote:
>>> Don Y <blockedofcourse@foo.invalid> wrote:
>>>> On 10/24/2021 4:14 AM, Dimiter_Popoff wrote:
>>>>>> Disable interrupts while accessing the fifo. you really have to.
>>>>>> alternatively you'll often get away not using a fifo at all,
>>>>>> unless you're blocking for a long while in some part of the code.
>>>>>
>>>>> Why would you do that. The fifo write pointer is only modified by
>>>>> the interrupt handler, the read pointer is only modified by the
>>>>> interrupted code. Has been done so for times immemorial.
>>>>
>>>> The OPs code doesn't differentiate between FIFO full and empty.
>>>
>>> If you read carefuly what he wrote you would know that he does.
>>> The trick he uses is that his indices may point outside buffer:
>>> empty is equal indices, full is difference equal to buffer
>>
>> Doesn't matter as any index can increase by any amount and
>> invalidate the "reality" of the buffer's contents (i.e.
>> actual number of characters that have been tranfered to
>> that region of memory).
> 
> AFAIK OP considers this not a problem in his application.

And I don't think I have to test for division by zero -- as
*my* code is the code that is passing numerator and denominator
to that operator, right?

Can you remember all of the little assumptions you've made in
any non-trivial piece of code -- a week later?  a month later?
6 months later (when a bug manifests or a feature upgrade
is requested)?

Do not check the inputs of routines for validity -- assume everything is
correct (cuz YOU wrote it to be so, right?).

Do not handle error conditions -- because they can't exist (because
you wrote the code and feel confident that you've anticipated
every contingency -- including those for future upgrades).

Ignore compiler warnings -- surely you know better than a silly
"generic" program!

Would you hire someone who viewed your product's quality (and
your reputation) in this regard?

> Of course, if such changes were a problem he would need to
> add test preventing writing to full buffer (he already have
> test preventing reading from empty buffer).
> 
>> Buffer size is 128, for example.  in is 127, out is 127.
>> What's that mean?
> 
> Empty buffer.

No, it means you can't sort out *if* there have been any characters
received, based solely on this fact (and, what other facts are there
to observe?)

>>   Can you tell me what has happened prior
>> to this point in time?  Have 127 characters been received?
>> Or, 383?  Or, 1151?
> 
> Does not matter.

Of course it does!  Something has happened that the code MIGHT have
detected in other circumstances (e.g., if uart_task had been invoked
more frequently).  The world has changed and the code doesn't know it.
Why write code that only *sometimes* works?

>> How many characters have been removed from the buffer?
>> (same numeric examples).
> 
> The same as has been stored.  Point is that received is
> always bigger or equal to removed and does not exceed
> removed by more than 128.  So you can exactly recover
> difference between received and removed.

If it can wrap, then "some data" can look like "no data".
If "no data", then NOTHING has been received -- from the
viewpoint of the code.

Tell me what prevents 256 characters from being received
after .in (and .out) are initially 0 -- without any
indication of their presence.  What "limits" the difference
to "128"?  Do you see any conditionals in the code that
do so?  Is there some magic in the hardware that enforces
this?

This is how you end up with bugs in your code.  The sorts
of bugs that you can witness -- with your own eyes -- and
never reproduce (until the code has been released and
lots of customers' eyes witness it as well).

>> The biggest practical limitation is that of expectations of
>> other developers who may inherit (or copy) his code expecting
>> the FIFO to be "well behaved".
> 
> Well, personally I would avoid storing to full buffer.  And
> even on small MCU it is not clear for me if his "savings"
> are worth it.  But his core design is sound.
> 
> Concerning other developers, I always working on assumption
> that code is "as is" and any claim what it is doing are of
> limited value unless there is convincing argument (proof
> or outline of proof) what it is doing.

Ever worked on 100KLoC projects?  500KLoC?  Do you personally examine
the entire codebase before you get started?  Do you purchase source
licenses for every library that you rely upon in your design?
(or, do you just assume software vendors are infallible?)

How would you feel if a fellow worker told you "yeah, the previous
guy had a habit of cutting corners in his FIFO management code"?
Or, "the previous guy always assumed malloc would succeed and
didn't even build an infrastructure to address the possibility
of it failing"

You could, perhaps, grep(1) for "malloc" or "FIFO" and manually
examine those code fragments.  What about division operators?
Or, verifying that data types never overflow their limits?  Or...

> Fact that code
> worked well in past system(s) is rather unconvincing.
> I have seen small (few lines) pieces of code that contained
> multiple bugs.  And that code was in "production" use
> for several years and passed its tests.
> 
> Certainly code like FIFO-s where there are multiple tradeofs
> and actual code tends to be relatively small deserves
> examination before re-use.

It's not "FIFO code".  It's a UART driver.   Do you examine every piece
of code that might *contain* a FIFO?  How do you know that there *is* a FIFO
in a piece of code -- without manually inspecting it?  What if it is a
FIFO mechanism but not explicitly named as a FIFO?

One wants to be able to move towards the goal of software *components*.
You don't want to have to inspect the design of every *diode* that
you use; you want to look at it's overall specifications and decide
if those fit your needs.

Unlikely that this code will describe itself as "works well enough
SOME of the time..."

And, when/if you stumble on such faults, good luck explaining to
your customer why it's going to take longer to fix and retest the
*existing* codebase before you can get on with your modifications...

Don Y <blockedofcourse@foo.invalid> wrote:
> On 10/26/2021 5:20 PM, antispam@math.uni.wroc.pl wrote:
> > Don Y <blockedofcourse@foo.invalid> wrote:
> >> On 10/25/2021 2:32 PM, antispam@math.uni.wroc.pl wrote:
> >>> Don Y <blockedofcourse@foo.invalid> wrote:
> >>>> On 10/24/2021 4:14 AM, Dimiter_Popoff wrote:
> >>>>>> Disable interrupts while accessing the fifo. you really have to.
> >>>>>> alternatively you'll often get away not using a fifo at all,
> >>>>>> unless you're blocking for a long while in some part of the code.
> >>>>>
> >>>>> Why would you do that. The fifo write pointer is only modified by
> >>>>> the interrupt handler, the read pointer is only modified by the
> >>>>> interrupted code. Has been done so for times immemorial.
> >>>>
> >>>> The OPs code doesn't differentiate between FIFO full and empty.
> >>>
> >>> If you read carefuly what he wrote you would know that he does.
> >>> The trick he uses is that his indices may point outside buffer:
> >>> empty is equal indices, full is difference equal to buffer
> >>
> >> Doesn't matter as any index can increase by any amount and
> >> invalidate the "reality" of the buffer's contents (i.e.
> >> actual number of characters that have been tranfered to
> >> that region of memory).
> > 
> > AFAIK OP considers this not a problem in his application.
> 
> And I don't think I have to test for division by zero -- as
> *my* code is the code that is passing numerator and denominator
> to that operator, right?

Well, I do not test for zero if I know that divisor must be
nonzero.  To put it differently, having zero in such place
is a bug and there is already enough machinery so that
such bug will not remain undetected.  Having extra test
adds no value.

OTOH is zero is possible, then handling it is part of program
logic and test is needed to take correct action.

> Can you remember all of the little assumptions you've made in
> any non-trivial piece of code -- a week later?  a month later?
> 6 months later (when a bug manifests or a feature upgrade
> is requested)?

Well, my normal practice is that there are no "little assumptions".
To put it differently, code is structured to make things clear,
even if this requires more code than some "clever" solution.
There may be "big assumptions", that is highly nontrivial facts
used by the code.  Some of them are considered "well known",
with proper naming in code it is easy to recall them years later.
Some deserve comments/referece.  In most of may coding I have
pretty comfortable situation: for human there is quite clear
what is valid and what is invalid.  So code makes a lot of
effort to handle valid (but possibly quite unusual) cases

> Do not check the inputs of routines for validity -- assume everything is
> correct (cuz YOU wrote it to be so, right?).

Well, correct inputs are part of contract.  Some things (like
array indices inside bounds) are checked, but in general you can
expect garbage if you pass incorrect input.  Most of my code is
of sort that called routine can not really check validity of input
(there are complex invariants).  Note: here I am talking mostly
about my non-embedded code (which is majority of my coding).
In most of may coding I have pretty comfortable situation: for
human there is quite clear what is valid and what is invalid.
So code makes a lot of effort to handle valid (but possibly quite
unusual) cases.  User input is normally checked to give sensible
error message, but some things are deemed to tricky/expensive
to check.  Other routines are deemed "system level", and here
there us up to user/caller to respect the contract.

My embedded code consists of rather small systems, and normally
there are no explicit validity checks.  To clarify: when system
receives commands it recognizes and handles valid commands.
So there is implicit check: anything not recognized as valid
is invalid.  OTOH frequently there is nothing to do in case
of errors: if there are no display to print error message,
no persistent store to log erreor and shuting down is not helpful,
then what else potential error handler would do?

I do not check if 12-bit ADC really returns numbers in range.
My 'print_byte' routine takes integer argument and blindly
truncates it to 8-bit without worring about possible
spurious upper bits.  "Safety critical" folks my be worried
by such practice, but my embedded code is fairly non-critical.

> Do not handle error conditions -- because they can't exist (because
> you wrote the code and feel confident that you've anticipated
> every contingency -- including those for future upgrades).
> 
> Ignore compiler warnings -- surely you know better than a silly
> "generic" program!
> 
> Would you hire someone who viewed your product's quality (and
> your reputation) in this regard?

Well, you do not know what OP code is doing.  I would prefer
my code to be robust and I feel that I am doing resonably
well here.  OTOH, coming back to serial comunication, it
is not hard to design communication protocal such that in
normal operation there is no possibility for buffer
overflow.  It would still make sense to add a single line
to say drop excess characters.  But it does not make
sense to make big story of lack of this line.  In particular
issue that OP wanted to discuss is still valid.

> > Of course, if such changes were a problem he would need to
> > add test preventing writing to full buffer (he already have
> > test preventing reading from empty buffer).
> > 
> >> Buffer size is 128, for example.  in is 127, out is 127.
> >> What's that mean?
> > 
> > Empty buffer.
> 
> No, it means you can't sort out *if* there have been any characters
> received, based solely on this fact (and, what other facts are there
> to observe?)

Of course you can connect to system and change values of variables
in debugger, so specific values mean nothing.  I am telling
you what to protocal is.  If all part of system (including parts
that OP skipped) obey the protocal, then you have meaning above.
If something misbehaves (say cosmic ray flipped a bit), it does
not mean that protocal is incorrect.  Simply _if_ probability
of misbehaviour is too high you need to fix the system (add
radiation shielding, appropiate seal to avoid tampering with
internals, extra checks inside, etc).  But what/if to fix
something is for OP to decide.

> >>   Can you tell me what has happened prior
> >> to this point in time?  Have 127 characters been received?
> >> Or, 383?  Or, 1151?
> > 
> > Does not matter.
> 
> Of course it does!  Something has happened that the code MIGHT have
> detected in other circumstances (e.g., if uart_task had been invoked
> more frequently).  The world has changed and the code doesn't know it.
> Why write code that only *sometimes* works?

All code works only sometimes.  Parafrazing famous answer to
Napoleon: fisrt you need a processor.  There are a lot of
conditons so that code works as intended.  Granted, I would
not skip needed check in real code.  But this is obvious
thing to add.  You are somewhat making OP code as "broken
beyond repair".  Well, as discussion showed, OP had problem
using "volatile" and that IMHO is much more important to
fix.

> >> How many characters have been removed from the buffer?
> >> (same numeric examples).
> > 
> > The same as has been stored.  Point is that received is
> > always bigger or equal to removed and does not exceed
> > removed by more than 128.  So you can exactly recover
> > difference between received and removed.
> 
> If it can wrap, then "some data" can look like "no data".
> If "no data", then NOTHING has been received -- from the
> viewpoint of the code.
> 
> Tell me what prevents 256 characters from being received
> after .in (and .out) are initially 0 -- without any
> indication of their presence.  What "limits" the difference
> to "128"?  Do you see any conditionals in the code that
> do so?  Is there some magic in the hardware that enforces
> this?

That is the protocol.  How to avoid violation is different
matter: dropping characters _may_ be solution.  But dropping
characters means that some data is lost, and how to deal
with lost data is different issue.  As is OP code will loose
some old data.  It is OP problem to decide which failure
mode is more problematic and how much extra checks are
needed.

> This is how you end up with bugs in your code.  The sorts
> of bugs that you can witness -- with your own eyes -- and
> never reproduce (until the code has been released and
> lots of customers' eyes witness it as well).

IME it is issues that you can not prodict that catch you.
The above is obvious issue, and should not be a problem
(unless designer is seriously incompenent and misjudged
what can happen).

> >> The biggest practical limitation is that of expectations of
> >> other developers who may inherit (or copy) his code expecting
> >> the FIFO to be "well behaved".
> > 
> > Well, personally I would avoid storing to full buffer.  And
> > even on small MCU it is not clear for me if his "savings"
> > are worth it.  But his core design is sound.
> > 
> > Concerning other developers, I always working on assumption
> > that code is "as is" and any claim what it is doing are of
> > limited value unless there is convincing argument (proof
> > or outline of proof) what it is doing.
> 
> Ever worked on 100KLoC projects?  500KLoC?  Do you personally examine
> the entire codebase before you get started?

Of course I do not read all code before start.  But I accept
risc that code may turn out to be faulty and I may be forced
to fix or abandon it.  My main project has 450K wc lines.
I know that parts are wrong and I am working on fixing that
(which will probably involve substantial rewrite).  I worked
a little on gcc and I can tell you that only sure thing in
such projects is that there are bugs.  Of course, despite
bugs gcc is quite useful.  But I also met Modula 2 compiler
that carefuly checked programs for violation of language
rules, but miscompiled nested function calls.

>  Do you purchase source
> licenses for every library that you rely upon in your design?
> (or, do you just assume software vendors are infallible?)

Well, for several years I work exclusively with open source code.
I see a lot of defects.  While my experience with comercial codes
is limited I do not think that commercial codes have less defects
than open source ones.  In fact, there are reasons to suspect
that there are more defects in commercial codes.

> How would you feel if a fellow worker told you "yeah, the previous
> guy had a habit of cutting corners in his FIFO management code"?
> Or, "the previous guy always assumed malloc would succeed and
> didn't even build an infrastructure to address the possibility
> of it failing"

Well, there is a lot of bad code.  Sometimes best solution is simply
to throw it out.  In other cases (likely in your malloc scenario above)
there may be simple workaround (replace malloc by checking version).

> You could, perhaps, grep(1) for "malloc" or "FIFO" and manually
> examine those code fragments.

Yes, that one of possible appraches.

>  What about division operators?

I have a C parser.  In desperation I could try to search parse
tree or transform program.  Or, more likely decide that program
is broken beyond repair.

> Or, verifying that data types never overflow their limits?  Or...

Well, one thing is to look at structure of program.  Code may
look complicated, but some programs are reasonably testable:
few random inputs can give some confidence that "main"
execution path computes correct values.  Then you look if
you can hit limits.  Actually, much of my coding is in
arbitrary precision, so overflow is impossible.  Instead
program may run out of memory.  But there parts for speed
use fixed precision.  If I correctly computed limits
overflow is impossible.  But this is big if.

> > Fact that code
> > worked well in past system(s) is rather unconvincing.
> > I have seen small (few lines) pieces of code that contained
> > multiple bugs.  And that code was in "production" use
> > for several years and passed its tests.
> > 
> > Certainly code like FIFO-s where there are multiple tradeofs
> > and actual code tends to be relatively small deserves
> > examination before re-use.
> 
> It's not "FIFO code".  It's a UART driver.   Do you examine every piece
> of code that might *contain* a FIFO?  How do you know that there *is* a FIFO
> in a piece of code -- without manually inspecting it?  What if it is a
> FIFO mechanism but not explicitly named as a FIFO?
> 
> One wants to be able to move towards the goal of software *components*.
> You don't want to have to inspect the design of every *diode* that
> you use; you want to look at it's overall specifications and decide
> if those fit your needs.

Sure, I would love to see really reusable components.  But IMHO we
are quite far from that.  There are some things which are reusable
if you accept modest to severe overhead.  For example things tends
to compose nicely if you dynamically allocate everything and use
garbage collection.  But performace cost may be substantial.
And in embedded setting garbage collection may be unacceptable.
In some cases I have found out that I can get much better
speed joing things that could be done as composition of library
operations into single big routine.  In other cases I fixed
bugs by replacing composition of library routines by a single
routine: there were interactions making simple composition
incorrect.  Correct alterantive was single routine.

As I wrote my embedded programs are simple and small.  But I
use almost no external libraries.  Trying some existing libraries
I have found out that some produce rather large programs, linking
in a lot of unneeded stuff.  Of course, writing for scratch
will not scale to bigger programs.  OTOH, I feel that with
proper tooling it would be possible to retain efficiency and
small code size at least for large class of microntroller
programs (but existing tools and libraries do not support this).

> Unlikely that this code will describe itself as "works well enough
> SOME of the time..."
> 
> And, when/if you stumble on such faults, good luck explaining to
> your customer why it's going to take longer to fix and retest the
> *existing* codebase before you can get on with your modifications...

Commercial vendors like to say how good their progam are.  But
market reality is that program my be quite bad and still sell.

-- 
                              Waldek Hebisch