Function prefix comments in C files

To be entirely honest, that sounds utterly horrible. It breaks all rules of modularisation and scope, and misuses C features. Your "Global" macro has different definitions in different files - never a good idea. C has an almost complete lack of proper modularisation features - if you want to write proper modular programs, you have to do it yourself.

The most common way to structure your files is one header file plus one source file equals one module, or is some variation on this theme. Each module exports an interface, consisting of functions, types, global variables, #defines, etc. Everything that is exported is in the header file, using "extern" declarations. The definitions are all in the source code file, along with non-exported data and functions (which should of preference be declared static, but most people are a bit sloppy about that). If you have a module called "timers", you might have a pair of files such as:

/* Timers.h */ #ifndef __timers_h__ #define __timers_h__

#include "common.h" extern volatile uint16 seconds; extern void initTimers(void); #endif

/* Timer.c */ #include "timers.h" volatile uint16 seconds;

static uint timerTicks; // Local data static void timerInterrupt(void) {... } // Local function

void initTimers(void) { ... }

There are a few things to note here. You need a "#ifndef ... #endif" pair to protect the header against multiple inclusion. The timers.c file includes the timers.h file - this allows the compiler to check for inconsistencies. You typically have a couple of stand-alone header files with common types or macros - but no "extern" data or functions without a corresponding source file to define them. There may also be stand-alone header files for libraries (either for pre-generated library files, or for separate directories of source files), and there will be one stand-alone source file containing your main() function.

Think of header files as declaring an interface, and source files as defining the implementation. That way you know what different bits of your program do, and you can find the bits you need, and you change only relevant bits of code. You can also re-use bits as necessary in different projects. Comments should, of course, match this separation - header file comments say what a function/variable does, and source file comments say how they do it (for source-local data and functions, they must say "what" as well as "how").

Reply to
David Brown
Loading thread data ...

I learned long ago that external documentation tends not to be maintained. I have even delivered projects consisting of high-level language files, and hand-compiled assembler files. Over time the customer has hacked the assembler files, and not maintained the high-level language files... gah. Wrong/obsolete documentation is, IMO, worse than no documentation.

Nowadays I try to keep all the code self-documenting, and even then I try to make the code so clear that extensive documentation is simply unnecessary. Exceptionally I'll provide a document outlining a particularly fiendish algorithm, but I have no great hopes that it'll be maintained.

Steve

formatting link

Reply to
Steve at fivetrees

snip... end snip...

snip... endsnip...

I strongly agree with David over this. However, I do not believe he has gone far enough. Global variables are **bad news**! I've seen so many problems with their use over a long career, that I eventually decided to stop using them altogether. For about the last ten years, I have never used a global variable in any new code I have written and I have tried to eliminate them from existing code wherever I can do so without undue disruption. It is always possible to design and code without global variables; if "efficiency" is a real worry (rather than just a perceived one) then, if possible, use a C99 compiler and "inline" simple data access functions. Using a functions - whether inline or not - to access data allows at least some of the necessary protections to be incorporated when the data items are shared.

Where globals are regrettably present, it is reasonable to gather them together in one file. After all, they are global and, by implication, belong to no particular module. And you can easily find them when the bugs bite. The GENERATING_GLOBALS trick, above, then works well. I even used it myself, in the old days! However, the simple, single-threaded, polling application, where this design model has its roots, all to often gets "improved" by the addition of a few interrupts. That's when the trouble really starts! The horrors of the actual design, though, become apparent only when someone says: "We need more responsiveness and more concurrency; let's add an RTOS". At that point it is quickly realised that the legacy code must be scrapped and a complete redesign undertaken. Proper modular design (with no global variables) in the first place would have meant more reuse of old code in the new system, even if only in the pragmatic "copy-and-paste" sense.

-- Peter Bushell

formatting link

Reply to
Peter Bushell

Yep.

I agree if you are talking about the implementation file (e.g. globals.c), but I do not agree if you are talking about a "globals.h" header file. Doing so potentially creates dependencies where there are none otherwise.

--
Michael N. Moran           (h) 770 516 7918
5009 Old Field Ct.         (c) 678 521 5460
 Click to see the full signature
Reply to
Michael N. Moran

Why must I search a *bunch* of files? I know that all prototypes are in prototype.h and all macros are in defines.h and all globals are in globals.h...?

Bo

Reply to
Bo

"Peter Bushell" wrote in message news:43464d78 snipped-for-privacy@mk-nntp-2.news.uk.tiscali.com...

How would you, in C, NOT use a global in the following circumstance?

100us counter incremented by an ISR access needed to read us counter by ALL modules?

When you are dealing with time critical code, you don't have the luxury of accessor functions. When you have very limited resources (RAM/ROM), ditto. When you have a VERY limited stack? or no real C stack at all? (ie the stack is emulated by use of registers by the compiler)

I'm not going to argue the point that this is not the preferred/std method of writing C. I know how and do use those methods David described when the luxury of ample RAM, ROM is available. But the method I described, though admittedly dated, does have its place--even today.

I don't know if it is still available, but Intel' FAX back service bulletin

3272 on their 80C196 compiler is the document that describes in detail some of the reasons why this was created. I had forgotten that one reason was access mode of the C196 and compiler causes nearby variables to be trashed if this method is NOT used. Since Intel no longer has this compiler (perhaps sold off to Tasking? I think), this may not be available any longer. Whether or not the underlying cause/issue has been fixed since that time I have no idea. I would hope that is has.

What troubles? This is exactly the kind of project is use this for and have never had any issues related to this method. Note that when dealing with the ISRs though ANY and ALL variables modified by ISRs must have the volatile modifier in the global declaration. My last project had 8 ISRs and had no issues at all.

Peter, I respectfully disagree.

Bo

Reply to
Bo

I agree that global variables should be avoided of preference, but I would not go to the lengths you suggest. They often provide the most convenient interface between parts of the program, and using inline functions for access is not always a good alternative. There are a number of reasons for that. First off, not all compilers are C99 or have decent inline functions. Second, adding "access functions" to your headers adds unnecessary bulk reducing readability. Third, access functions don't actually hide the globals - they still need to be visible to the access functions. Fourth, if the data is structured, access functions get much less efficient, much more verbose, and far less readable.

It is important to know who (i.e., which module and/or functions) have rights and responsibilities regarding each global variable. Is the variable read-only outside the module? Is it read/write at any time, or only at specific times? Should it be volatile? When is it initialised? These are all important questions, and can only be enforced to a limited degree by access functions, and it is sometimes at the cost of efficiency (and often at the cost of readability). The most important ways to handle these issues, whether you want direct access to globals or to use access functions, are modularity, structure, and consistency. If a global variable "seconds" is defined in the "timers" module, then you know exactly how it is initialised - by the timers module. You know what module has responsibility - the timers module. You know where to look for comments regarding its use - the timers module. There are certainly times when an access function is the right way, such as when there must be some locking or synchronising, but dropping all globals is overkill.

Reply to
David Brown

Still better, put ALL the code in one file. Preferably without comments.

Paul Burke

Reply to
Paul Burke

I generally agree that global variables can be safely used, and I have an aversion to replacing "thinking" (What's the best way to design this software?) with "rules" (No gobal variables allowed!). But your example is not the best.

Consider an 8-bit microprocessor. If your counter is wider than one byte (which would not be uncommon for a 100usec counter), then an accessor function can not only save space, but will help prevent counter read errors when the programmer forgets to disable interrupts. Function call overhead is not as great a penalty as many seem to think it is, especially when no parameters are passed.

That said, in my 8-bit code, all my free-running timers are single-byte global variables, though the smallest resolution is 1msec.

Regards,

-=Dave

--
Change is inevitable, progress is not.
Reply to
Dave Hansen

I see 2 lines of comment with 3 lines of 'code' inbetween, not 5 lines of comment as you probably intended.

--
Stef    (remove caps, dashes and .invalid from e-mail address to reply by mail)

It is easier to change the specification to fit the program than vice versa.
Reply to
Stef

I'm also anti-globals, but by "globals" I mean "global to all modules". Within a given module (i.e. a .c file), I do allow globals - i.e. variables that are within scope for that module, but out of scope to all others. (To be precise, I usually define a class - actually a structure - which contains any variables which are in-scope within that module.)

Accessor functions can then be of project-global scope, but provide real protection: a read function can *only* read, while a write function (if required) does so in a strictly controlled manner.

I haven't used an actual global global in maybe 15 years...

Steve

formatting link

Reply to
Steve at fivetrees

Paul,

Do detect a wee bit of sarcasm here?

Either that, or you are perhaps like a LOT of EE's that write software I've seen :)....hopefully not!

No offense to EEs!---because that's what I am too! But my view of most EE's code is less than favorable. Hap-hazardly designed, full of goto statements and no comments at all. I've actually seen some real life code that could've made it to the

formatting link
obfuscated code pages without too much stretch! I ABHOR unreadable, unmaintainable code. This code I'm referring to, the guy actual inserted NO carriage returns and the editor wrapped around after column 256 or something ridiculous. No thanks!

If there's anything anyone can say about my code, it is that it is readable and straight-forward. No C tricks, absolutely NO gotos (sounding like Peter regarding globals now, am I not?) and plenty of USEFUL comments.

I also encountered someone who advocated the opposite of your suggestion. They were tasked to re-write some C code I had into C++ and add a few new features. Result? My 2000 lines of code in 10 C and 3 h files, turned into

140 files (each with one and only one function--even if it was only a 3 liner) and it would not run. I ended up re-taking over the project, modifying my original code for the new features, converting to C++ and I still had only 10 files and perhaps an additional 2 header files---and the code worked :).

Regards,

Bo

Reply to
Bo

So your badly structured code is not quite as bad as someone else's code, and that makes it all right? Your 2000 lines of code should be structured as approximately 10 .c files and 10 .h files.

Reply to
David Brown

Nothing in the methods I described is going to cost you in terms of RAM, ROM, or run-time. It's nothing more nor less than a logical re-organisation of the same code you've been writing. Trying to eliminate all globals by forcing the use of access functions is a different matter - sometimes it can be done "free", sometimes it will cost.

You are telling me that you base your style of programming on an outdated workaround for a badly broken compiler? Sometimes in this industry there is no choice but to use broken or inferior tools, but that's no excuse for not writing proper code in other circumstances.

The idea that "ANY and ALL variables modified by ISRs must have the volatile modifier" is a common misconception, and shows an incomplete understanding of what "volatile" means, and how ISRs interact with other code. Although it is safer to have a few too many "volatile" modifiers than too few, using them blindly shows you haven't thought through the possible interactions, and you could also miss other issues (for example, many people mistakenly think "volatile" implies "atomic").

Globals can often be the cause of problems if they are misused or misunderstood. This should be solved by proper program structure, comments as appropriate, and being clear over how the variables are to be used. Access functions can enforce some control, but not much.

Reply to
David Brown

The counter would be static. The ISR would be in the same file and would access the static variable directly. Access functions (public, global, non-static - whatever you wish to call them) would also be provided, in that file, for use by other modules. Each of these access functions would disable interrupts around the access. The associated header file would publish these functions. It would probably be necessary to publish the ISR, also, so that its vector could be set up.

I hope that helps!

-- Peter Bushell

formatting link

Reply to
Peter Bushell

(snip)

(snip)

I was a bit glib about access functions. They can be used to access static variables but, of course, the functions cannot then be inline. Inline functions are useful for accessing structure members, given a pointer to the structure - do you sense some OO going on here? - but the structure declaration itself must be visible to the compiler at the time. Pity the C99 people didn't add the class concept of privacy at the same time as they added "inline"!

-- Peter Bushell

formatting link

Reply to
Peter Bushell

Good. What you described, however, are execeptions to the rule ... and there are *always* exceptions. However, the *rule* from a portability and maintainability point-of-view is best served as Peter suggests.

Agreed, however, that in a resource constrained environment, execeptions are frequently legion.

--
Michael N. Moran           (h) 770 516 7918
5009 Old Field Ct.         (c) 678 521 5460
 Click to see the full signature
Reply to
Michael N. Moran

That's all very nice, but it can't be done efficiently. In the world of small embedded systems, using an access function like this takes too long for something that should be a simple read from memory (note - in the special case of such a counter, non-atomic access might make this particular global a good candidate for an access function. I'm talking about general cases). Either you use inline functions (which then need access to the global variable, so you are not achieving anything), or your code efficiency suffers badly. You cannot expect to enforce anything but the simplest of access controls by using access functions, and if you or your fellow programmers are incapable of following comment instructions like "this variable is read-only outside this module", then they are in the wrong job. If you are talking about a large collaboration effort with programmers from around the world, then there is a lot of sense in trying to enforce tight interfaces - and it is done by using a language other than C. This is comp.arch.embedded - programs are mostly written by individuals or small groups, with any code from other sources carefully vetted before inclusion. Non-inlined function call overheads are just too large for many uses (as well as the call-return overhead, they completely mess up your compiler's register allocations and optimisations), and the syntax messes up the readability of your code. This is especially true for arrays and struct variables.

Reply to
David Brown

"Peter Bushell" wrote in message news:4346b20e snipped-for-privacy@mk-nntp-2.news.uk.tiscali.com...

Important postscript...

Disabling interrupts around individual reads and writes is OK as far as it goes. Some people don't even do that, arguing that accesses to integers, say, are atomic. Well I'm more paranoid than that. Can we be sure that the compiler will generate a single instruction? Even if the structure containing the integer is packed? Even when someone decides to make the integer a long (or a long long)?

Anyway, none of this helps if you're doing non-trivial read-modify-write operations. Then you have to do the protection round the whole operation. That's what I meant when I said that access functions can provide "at least some" of the required protection. Many applications, though, do simple reads and writes of buffers, which are shared between an ISR and a task (or main program). The r-m-w operations tend to be limited to things like incrementing a counter or bumping a pointer, and such operations can themselves be fully contained within their own access functions.

-- Peter Bushell

formatting link

Reply to
Peter Bushell

So you search them one at a time, for different things (except for the global variable question, where you may not find it.) It's still a bunch of files. I much prefer one, where I not only can get the same information, but add information on the relationship between things.

--
Al Balmer
Balmer Consulting
 Click to see the full signature
Reply to
Alan Balmer

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.