C Question

[UtopiaTim]

Hi Folks,

Being a noob to C, and only having a little hair left, here's a question.

Simple decision box:

Check to see if location Key_temp is = 0.  If not, wait till it changes (done via I-rupt routine).

When it becomes non-zero, then check Month_current to see if it is 12.  If so,

then make it 01, then go do some housekeeping.

First, I tried with only the 1st comparison to Key_temp, & here's what I got:

C code:                                                                                            Assembly

void do_month(void){                                                                     199A  TST  0xB5

                                                                                                       199C Beq *-2      ;abs = 0x199A

    while (Key_temp == 0);   // wait until there is a key available    199E  NOP
                                                                                                       199F  RTS
    if(Key_temp==0x48);      //Up - short?           
       asm{
       nop
       }
}

The while instruction worked fine, but what happened to the if(Key....) assembly code?

In my real code, the first decision tree (Key_temp=48), should be followed by a second decision,

if  (Month_current=12).

When I do the following:

void do_month(void){                                                      199A  TST 0xB5

    while (Key_temp == 0);       // wait                              199C  BEQ *-2
                                                                                        199E  LDA 0xBC  (location Month_current)
    if(Key_temp==0x48);          //Up - short?                     19A0  CMP #0x0C

       if(Month_current==12){                                            19A2  BNE *-8    ;abs = 0x19AA
         Month_current=1;                                                   19A4  MOV #0x01,0xBC
         Update_month();                                                    19A7  JMP 0x19AB
       }                                                                                19AA  RTS
}

Figured I'd see what I'm doing wrong before I go nuts!!

Thanks,

Tim

[bigmac]

Hello Tim,

---

Utopia Tim wrote:

I ended up using the switch/case method, as I couldn't figure out how to get
the function to exit using the continue method.

I understand the switch/case method now, & that's good. (although it adds a
chunk of assembly code - glad this function isn't time sensitive).

I seem to recall that, in one of the forums, it was alluded that the CW compile of the switch/case construct (presumably for an 8-bit MCU) was inefficient, and I also recall a recommendation to use an if/else construct instead.

To explore the extent of this issue, I used the following code snippets for a simple state machine, that should produce identical results -

Sample 1:

byte n;

switch (n) {
case 0:
   func0( n);
   n = 2;
   break;

case 1:
   func1( n);
   n = 3;
   break;

case 2:
   func2( n);
   n = 1;
   break;

case 3;
   func3( n);
   n = 0;
   break;

default:
   n = 0;
}

Sample 2:

byte n;

if (n == 0) {
   func0( n);
   n = 2;
}
else if (n == 1) {
   func1( n);
   n = 3;
}
else if (n == 2) {
   func2( n);
   n = 1;
}
else if (n == 3) {
   func3( n);
   n = 0;
}
else  /* Default case */
   n = 0;

When each the program containing each snippet type was compiled, the interesting point was that both programs produced exactly equal code size.  However, when each program was tested using full chip simulation, in fact Sample 2 executed more quickly.  For each pass through the code, Sample 1 required between 64 and 79 cyles more than Sample 2, a significant amount for a relatively simple process.

I thought this might be of interest.

Regards,

Mac

[Lundin]

For this particular case when the numbers are 0 to n (or rather adjacent), you can optimize the code by declaring an array of function pointers instead.

#pragma INLINE
void func1 (void)
{

}

#pragma INLINE
void func2 (void)
{

}


int main()
{
  void(*func_array[N])(void) =
  {
    func1,
    func2,
    ...
  };


  func_array[n]();

}

[JimDon]

Well, ok, that gets rid of all control structures but what if you need to handle 10, 25, 48 and 65.
That will be a huge table, very prone to error. Not too safe. In fact, initializing  a table in this way is very prone to error, as it is all to easy to lose count
.
BTW I guess you'll still need startup.c to initialize the table.

Also, if you are calling them from a table, what is the point of inlining?

[Lundin]

As I wrote, they need to be adjacent. If they aren't, you'll have to use switch instead. You also need a fair amount of numbers to motivate the overhead of the function call.

I made the functions inlined out of old habit, really, as if they are called from a "real" switch statement.

The function pointer array should be const, I forgot that... modified code:

#pragma INLINE
static void func1 (void)
{

}

#pragma INLINE
static void func2 (void)
{

}


static const void(*func_array[N])(void) =
{
  func1,
  func2,
  ...
};


int main()
{
  func_array[n]();
}

[CompilerGuru]

is

static const void(*func_array[N])(void);

an array in the flash?

I think it should read as (but I did not try):

static void (*const func_array[N])(void);

For functions, I always use a typedef for arrays.
typedef

typedef void (*func_type)(void);
static const func_type func_array[N];

Daniel

[JimDon]

Daniel,
Well, you check for sure but the linker may not do the same thing for each of the declarations;.



static const void(*func_array[N])(void); // the table is in flash.

static void (*const func_array[N])(void); // the table is in ram, but you can't modify
                                          // any of the pointers. compiler enforced.


typedef void (*func_type)(void);
static const func_type func_array[N];

is the same as:

static const void(*func_array[N])(void);

[CompilerGuru]

Actually I was so surprised by this that I did try it out. Here's what I found:

For:

Code:

#define N 100const void(*func_array_0[N])(void);void (*const func_array_1[N])(void);typedef void (*func_type)(void);const func_type func_array_2[N];

 

With Smart-Linking off,
here's the map file content:

Code:

     func_array_0                               100      C8     200       0   .common          func_array_1                              E092      C8     200       0   .rodata          func_array_2                              E15A      C8     200       0   .rodata    

 

So func_array_0 is in RAM and func_array_1 and func_array_2 is in flash.

In my understanding, for pointers, including arrays of pointers, the const qualifier should be after the * to be unique. A const does at the same time tell the compiler to disallow runtime changes and does also cause that by default such objects end up in flash. Although the latter maybe overridden in the prm file, just as you can explicitly place non constant objects in flash too via prm and explicit section.

Daniel

---

JimDon wrote:
Daniel,
Well, you check for sure but the linker may not do the same thing for each of the declarations;.



static const void(*func_array[N])(void); // the table is in flash.

static void (*const func_array[N])(void); // the table is in ram, but you can't modify
                                          // any of the pointers. compiler enforced.


typedef void (*func_type)(void);
static const func_type func_array[N];

is the same as:

static const void(*func_array[N])(void);

---

[JimDon]

Actually, it is correct for pointers, except when using the type def, which I would call a bug.
This is the logic:

Normal "C" usage:
const int* p;        // can't change what it points to p should be in RAM.
int* const p;        // can't change p should be in flash.
int const* p;        // same thing.
const int* const p;  // can use both. p should be in flash.

[allawtterb]

In your list JimDon, the third case should be the same as the first case.  All that matters is where the asterisk in relation to the declaration specifiers. Also, cases 2 and 4 must initialized when declared because they have constant pointers, once declared they can't be changed.

[JimDon]

You know what, you are 100% correct. Sorry.

const int* p;        // can't change what it points to p should be in RAM.
int const* p;        // same thing.

int* const p=0;        // can't change p should be in flash. Must Init now.
const int* const p=0;  // can use both. p should be in flash.Must Init now.

[CompilerGuru]

For me the typedef behavior makes sense, not sure where you see a bug there.
That the const applies to the complete type and not to the token thing in the typedef definition is what I expect, that's actually one of the points why I prefer typedefs over macros for such things.

Also object pointers look a lot simpler than function pointers (without typedef) :smileyhappy:.

const's should be reasonably initialized in C, but the standard does not require it. For C++, the initialization value of consts is mandatory.

Daniel

[bigmac]

Hello,

The use of a table of function pointers was suggested as an alternative to the use of "traditional" program flow control methods, such as switch/case and if/else methods.  I would agree that the table of function pointers method will probably be very efficient.  It would seem to relate very closely to straight forward assembly language processes using a table of sub-routine addresses.  However, as the previous discussion indicates, in C it is a rather more arcane process, probably not suited to the less experienced programmer.  For C, I would put myself in this category.

For persons, such as the OP, the use of the traditionall methods may be less prone to error because they are conceptually easier to understand IMHO.  I might also argue whether the use of the function pointer method would result in code that was easier to maintain in the future, than the alternatives.

So this brings me back to the previous criticism of the utilisation of the use of goto and continue keywords, and use of multiple break and return keywords.

Lundin wrote:

There are roughly four evil ways to "boil spaghetti" in C:

- goto (or asm JMP)
- continue
- More than one break in for/while loops.
- More than one return statement in functions.

All of them are considered poor programming-practice and are explicitly banned by coding standards such as MISRA-C. Writing "continue" instead of "goto" isn't cleaner just because you use another keyword. You shouldn't need to use either of them.

---

Multiple return statements:

I fail to understand why this might be considered poor programming practice.  Where ever it is used, the program flow will be unequivocal - the function will immediately exit.  If used to return, say an error status value, the return of a constant value at various locations within the function should actually be more efficient than assigning a constant value to a local variable, and eventually returning the contents of the variable.  It may also result in the use of fewer break statements.

Multiple break statements in for/while loops:

I notice that this ban is restricted to for/while loops only.  It is obviously essential for multiple break statements in code associated with a switch statement.  With any type of construct, the ease with which program flow can be followed will surely depend on the number of nesting levels used within the function.  Uncertainty is likely to be considerably enhanced by the use of break (and continue) statements at more than one nesting level, especially if there is any inconsistency of indentation within the source code.  If restricted to a single nesting level, and preferably one of the outer levels, I would consider the use of multiple breaks to be satisfactory.

I would consider a more important rule for the ease of following program flow,  to restrict the number of nesting levels within a function to perhaps no more than 3 or 4 levels.  If more levels seem to be necessary, perhaps the function is too complex, and additional functions need to be created to handle the inner workings.

Continue and goto statements:

Personally, I have not had the need to use either of these keywords.  I guess it is tempting to use goto as a method to quickly exit through a number of nested program blocks, and may actually be easier to follow than the use of break statements at multiple levels of nesting, and also assuming that the position of the label is within the same function.

Regards,

Mac

[JimDon]

No, no bug. It all as it should be.
I said that because I was thinking that
int const *p
made the pointer const.
My mistake, it is all fine.

[UtopiaTim]

Hi Mac,

Thanks for the input. 

Yes, I noticed that in the switch/case method, there was some additional

code that performed the actual comparisons & jump.  Probably accounts

for the extra cycles.

Fortunately in this case, it shouldn't be a problem with the keyboard input

routine, but it's a good thing to know.

Should it be an issue, I will re-visit at a later time.

Thanks again!

Tim

[UtopiaTim]

Hi Jim,

That fixed it.  Thanks a bunch for the help (everyone).

Yes, the variable started in a different file, and I had put an 'extern'

in the header of this file.  I'll check on the volatile issue, as it will be

changed by the interrupt service routine.

Thanks again!

Tim

[UtopiaTim]

//**********************************************************************************
//
//      MODE 4  setup Date and Med names/times
//
//**********************************************************************************
                                            
void Run_Mode4(void){

Do_month();
 
 
}  
 
 
void Do_month(void){

    while (Key_temp == 0);              // wait until there is a key available 
             
    if(Key_temp==0x48)                 //Up - short?           

//       if(Month_current==12){
//         Month_current=1;
//         Update_month();
//       }
}

Key_temp is a global variable.  This code will give the error.  If I replace the ;

after ..0x48), it will compile with no errors.

Thanks,

Tim

(I think this is how you wanted the code)

[UtopiaTim]

Hi Frank/Jim..

I removed the ; from the If line, and now I'm getting the following error message:

Error: C2450: Expected...... volatile while __asm __interrupt.  With the ;, it compiles (just

doesn't make code)!

Thanks,

Tim

[UtopiaTim]

Thanks JimD... I figured it was something with syntax... as it always seems to be.

I'll give it a try in a little while.

Thanks,

Tim