Function prefix comments in C files

Ahh, now that reminds me of a bug I spent an unreasonable time searching for (in a commercial RTOS). They were saving/restoring a pointer that was used to access some critical internal structure and the access was left unprotected. The chip I was using had, unfortuately, a veritable legion of bugs or so it seemed and so to keep matters simple I had turned off all the optimizations. It turned out that when the compiler had its default (or higher) optimization level on the access to the pointer was atomic. With optimization turned off access was no longer atomic and it turned into a Heisenbug showing up intermittantly and dissappearing as soon as any attempt was made to find it. It didn't help that the ICE had a different set of bugs than the production micro and required a different set of compiler switches.

When I finally found the bug and reported it to the developers they claimed that it wasn't a bug since it dissappeared when the optimization level was increased.

Annoyingly they had a #define in the code to turn on access protection around the access to this pointer but had left it undeffed (sp?) since they 'knew' the compiler would always produce atomic access to the pointer.

Robert

No, you mixed up posts (or their authors). I don't advocate dropping globals - I advocate a structured, modular layout of your files. The first poster bringing up the idea of "all globals are evil" was Peter Bushell, IIRC.

For small enough programs, it is easy to keep track of where things are. As programs get bigger, structure and organisation becomes more important. It's also more important when you have to go back to older code, and when other people have to look at the same code (that's where there are advantages of using standard methods - not that I think being "standard" is sufficient reason for picking a technique). It sounds to me like your and your colleagues are mainly electrical engineers, and none have any real training in programming (that's not a criticism in any way - we can't be experts in everything). Your methods are not scalable, and not modularised - but they could well still be good enough for your current needs, and better than your colleagues.

The key advantage of modular programming is, well, that it is modular. That makes it scalable, and (relatively) easy to re-use code in different projects. It means that parts of the code that are related by function are in the same place, rather than splitting your code according to the type of programming language construct.

You have, to a fair extent, answered the question yourself by adding the qualified "modified by an ISR and used in the program elsewhere". It also *may* apply to data that is modified by the program elsewhere, and used by the ISR. But volatile is not needed for data that is only used within the ISR, and there are plenty of cases where volatile is not the best way to deal with synchronising data between ISRs and background code (for example, if there is a lot of data, maybe a single volatile "update" flag can be used while the rest of the data is declared non-volatile).

Just as a quick example, suppose you have a timer interrupt running at

256 Hz, and you have a "seconds" counter that is read by other code. Then you might well have:

volatile uint16 seconds; uint8 partSeconds; void timerInterrupt(void) { if (!(++partSeconds)) seconds++; }

Keeping partSeconds non-volatile might make the interrupt code better, especially if it uses partSeconds for other purposes.

If you have another interrupt routine that needs to read both seconds and partSeconds, it can safely do so without the volatile (assuming you don't have nested interrupts).

My point here was that many people think that in the code above, it is safe to read "seconds" in the main code since it is declared volatile. If the target is an 8-bit cpu, then reading the 16-bit "seconds" variable can't be done atomically regardless of the volatile qualifier. This would be a case where an access function could be very useful rather than a global variable, with code like:

uint16 readSeconds(void){ uint16 sampleA = seconds; uint16 sampleB; while (true) { sampleB = seconds; if (sampleA == sampleB) return sampleA; sampleA = sampleB; }; }

I'm going to be on holiday for the next week, so if I don't respond to your posts, don't feel snubbed!

David

This will be dependant on your compiler-target combination. For example, on an 8-bit micro you can be sure that access to an int will

*not* be atomic (since an int is at least 16 bit). Thus you need to do something like disabling interrupts, or doing multiple reads, if reading the volatile data - if the read is not atomic. So for accessing multi-byte volatile data in a safe way, access functions can sometimes be a good solution (though not necessarily the best or most appropriate solution).

Of course you should use functions to ensure safe access to structures like buffers shared between ISRs and other parts of the program. I would not think of such data as "global" in the first place - it is private to the ISR's module. But supposing this module (say, a UART handler) also has a flag variable called "transmitting" that is declared as a volatile global variable. Only the UART module will ever change this flag, but other modules may want to read it. Where is the harm in exporting this as a global variable for all to read?

There's a lot that could be done to C to make it a better language, but it would probably be easier to start from scratch. Certainly if there were some way to give compiler-enforced safe and controlled access to global data without the overhead of functions, I'd use it - but there isn't. To paraphrase Winston Churchill, C is the worst possible language, but it's the best we've got.

BINGO! The first thing that came to my mind when I read Bo's description of header organization is that he must not be at all concerned about code reuse.

--
========================================================================
          Michael Kesti            |  "And like, one and one don't make
                                   |   two, one and one make one."
    mrkesti at comcast dot net     |          - The Who, Bargain

This is why using the newer generation 8-bits are better than using the older generation 8051, PICs etc. The performance and size impact is much smaller on the newer generation 8-bits when doing things "correctly" than for the older generation small MCUs. This is something which is often forgotten when the various pros and cons of using one or the other MCU are discussed in this forum.

Regards Anton Erasmus

You may someday need to use two or three bits for that purpose or include some other logic, as well. For example, expanding the features and functions of your UART handler and doing so without materially impacting the existing code that already does a good job. Let's say that in the process of doing so, another bit should be included in the meaning of 'transmitting'. You could, of course, redefine that original bit to include this new meaning, but that might get into some difficult ISR versus function call from a process timing problems that were already well worked out before but now become a new problem, if attempted. Or it might simply mean modifying code that has been well tested and works fine and thus risking what's already been well tested. So an additional bit might be the lower-risk/better way to go, but now because you've exported the bit for other code that option is closed to you. Having preferred a function to access the bit means that changes to your design can be a little more readily contained.

Anyway, there's a potential for 'harm.' Not that it's necessarily going to be a real one for any particular application.

Jon

Must - not - mention - favorite - alternate - language ;-)

-- Michael N. Moran (h) 770 516 7918

5009 Old Field Ct. (c) 678 521 5460 Kennesaw, GA, USA 30144

"So often times it happens, that we live our lives in chains and we never even know we have the key." The Eagles, "Already Gone"

The Beatles were wrong: 1 & 1 & 1 is 1

My apologies...to all.

David,

A good discussion. :) I look forward to your return and follow-up on this....

Your example of using a volatile flag (eg stale_data) to let other areas of program know they do/do not need to re-read data is a good case where declaring the entire data set volatile is not needed--and is a better choice I think. So I take it that my qualified explanation is accurate(---unless specific data structures with a volatile flag indicating their status is employed) on use of volatile.

Your quip "> The key advantage of modular programming is, well, that it is modular.

This could easily lead to another very good discussion. For example, do you group all ISRs in one module or group for example by peripheral? ie all uart0 functions and its ISR are in one file?

A hypothetical question... lets say I have a MCU with 5 timers. Code for all timers can be identical with adding timer number parameter for a given function.

Do you:

1) create 5 modules and duplicate the code 5 times? 2) use reentrancy (assuming timer ISRs and/or other code needs to call the function(s) ) and write your code for generic timer x, pass the timer number to the routine(s) and have only one timer module? 3)Other?

For a contrived example: #1

timer0_enable(char flag) { TIMER0_CTL = flag; }

timer1_enable(char flag) { TIMER1_CTL = flag; } ... timer4_enable(char flag) { TIMER4_CTL = flag; }

OR---#2

timer_enable(char flag, char timer) reentrant { *((unsigned char*)(TIMER0_CTL + timer*4)) = flag; }

where TIMER0_CTL is defined as an address.---like

#define TIMER0_CTL *(unsigned char*) 0xA000 // for the first case #define TIMER0_CTL 0xA000 // for 2nd case

This is a trivial example where we're not doing much in the given routine--but I hope it illustrates what I'm speaking of conceptually. Imagine that this routine is doing much, much more...

Each has its advantage/disadvantage. Use of reentrant functions can be troublesome/problematic with some tools. On the other hand, if you are running short of ROM or RAM/stack, you could possibly save a lot of space by going with reentrant (especially if the function has a lot a local variables). Or is there a better way altogether?

I would not use #2 if the bits/registers of the timers we're different 'enough' so that really ugly macros or lots of ugly handling of differences would be required. Kind of like some data sheets I've seen from Maxim that try to make 1 data sheet for 15 variants of parts...the end result being total confusion as to what parts of the data sheet apply to which device(s) (eg. MAX3372). It'd be a LOT simpler to just make 15 data sheets and be done with it. In this particular example, the affected medium is just size/number of files--not actually worry with 'fitting' something in a given space.

No offense on the EE/programmer business. I certainly am no expert on many features --especially of OO language like C++ (eg std template lib). That said, I think I have a fairly good handle on when/when not to use the 'std method' for modular coding.--and the likelihood of need for code re-use or porting. I think this has been a very good discussion. Thanks for taking part David--and Peter, et al.

OR #3--- your better way(s)????

Regards,

Bo

In one case, I used Other:

I had a processor with multiple serial ports, that behaved identically (or nearly identically). If I had a single driver and with a run-time selection of port number, it would substantially slow the code to do the run-time calculations for table addresses and port addresses, flags, etc. I effectively wrote a template in C, with a parameter of the port number (maybe other items, too -- I don't recall). The template was done by using preprocessor symbols for the port number (and possibly other port-related variables). The module for a specific port defined those preprocessor symbols, then included the common code file, which compiled optimized code for that port. Any computations, such as array subscripting on port number, was done at compile time, since it is constant. If you get fancy with macro processing, you can define a macro with the port number, then generate associated symbol names containing that name, such as the routine name, I/O port name, name bit mask, etc.

Simple example:

SerialDriver1.c:

#define PORTN 1 #define ISRN ISR1 #include "SerialCommon.ci" /* .ci indicates included C code */

SerialCommon.ci: void ISRN (void) { ... c = MMPortRegister[PORTN]; serialstat[PORTN].lastchar = c; ... intFlags &= ~(1

The answer is very much "it depends". I've had occasion to use all three methods (although in #2, you don't really mean "reentrancy" - you mean "abstraction". Your generic functions may need to be reentrant, if you have re-enabled interrupts, but that's not the main point here). Basically, it boils down to the similarity of the uses for the timers. If you have your five timers doing different specific jobs, then the code will be mostly separate, and they'll be called things like "fastTimer" or "refreshTimer", not "timer1" and "timer2". If they have identical functions, say, three pwm output timers, then they'd be called with functions like "setPwm(uint8 pwmNo, uint16 value)". Often such functions will be written as static inline functions declared in the header, so that the compiler can reduce the nice, abstract functional api to a couple of load instructions.

As for splitting them into modules, you (normally) do it by functionality in your program, not functionality of the underlying device. If you have a couple of timers used for software functions and three used for pwm outputs, you'd put the software timers in one (or two) modules, and the pwm outputs in another. If the underlying hardware requires extra abstraction, you'd put it in another module that is used by the timer modules (but not directly by any other modules in the system). That's the idea of splitting the program into tasks, and separating the tasks into modules. Divide and conquer!

mvh.,

David

Hi, David. I can't speak to specific situations, of course, as I can only speak to those I've enough knowledge about.

A designer must weigh various issues and none of what I said should change any of that. But I did point out a possible avenue for harm in exposing a bit to external use. This doesn't mean that I don't do that, myself, in some embedded apps. I was just calling one potential out onto the table for view _only_ and _because_ you specifically asked the question and invited a response.

Jon

I agree absolutely, and you are right - there are times when exposing a variable to external modules can be a problem. As we both know, the answer is always "it depends". My post was more an argument against the "never use globals, always use functions" crowd rather than the compromise of "use what is appropriate at the time, but consider future changes too".

mvh.,

David