Consider the function OALIoCtlHalInitRTC, lines 122 and 139-143. The last else block has no obvious terminator, but it looks like the else is empty because of the comment and the indentation. So the line after the label cleanUp is the body of the else. This means that if the if on line 131 is true, the cleanUp code never gets executed. Now this may be intentional (but I doubt it since the label is named cleanUp), but that should be documented in the code.

I consider this use of a goto to be legitimate, since it avoids duplication of the cleanup code (and the last thing you want is to have multiple similar blocks of code that need to be maintained/modified in tandem). This idiom often appears in the linux kernel IIRC. But this code is quite terrible since it doesn't use the idiom correctly, produces an unexpected execution path, is a problem to maintain, and is a problem to read. It seems that it ends up being a no-op since the line after the cleanUp label is a debugging line or something, but that doesn't make this a good practice.

But really, this isn't really a problem with the use of goto, it's a problem with the optional braces on single statement blocks in C. The goto and the label chosen just make it harder to spot. I've made it a habit to use braces around all blocks, even empty ones, to make the intent clearer and avoid this problem.

The rest reads "HAL (hardware abstraction layer) initialize RTC (real-time clock)".. and that's what it does.

This is from five minutes looking at the source. Very little college embedded programming. What more do you want?

// This function is called by WinCE OS to initialize the time after boot.

So how about calling it:

init_time

or maybe

init_time_after_boot

You're either going to have a naming conflict, or you're going to have a bunch of idiots who wanted to initialize a different clock, who start posting to forums saying "i don't understand???! i called init_time waht's wrong???"

Sure, they could call it

SendTheIOCTLCommandToInitializeTheRealTimeClockForTheOEMAbstractionLayer

but sometimes the names of functions are going to be ugly.

Isn't that the definition of "when to extract code into its own function"?

That is to say, I could have fixed the bug without knowing what it was. (I'm a hs student and don't do much bug-shooting in code that wasn't written by me or my close friends, so I don't know if this is common.)