C Question

UtopiaTim · ‎01-19-2008

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

allawtterb · ‎01-29-2008

I believe you are correct JimDon,

static void (*const func_array[N])(void); // Constant pointer to a variable object

as opposed to,

static const void(*func_array[N])(void); // Variable pointer to a constant object

JimDon · ‎01-19-2008

Code:

   if(Key_temp==0x48);          //Up - short—                     ^ --- Bug code below always executes.       if(Month_current==12){                         Month_current=1;                             Update_month();                            }            
Never write if(); or while();
Always write if()
               ;
   while()
     ;
I always write:
if()
{
   if()
   {
  
   }
}

Even if it "wastes" some white space, it make it easier to figure out later.

Message Edited by JimDon on 2008-01-19 02:42 PM

thisobj · ‎01-19-2008

Hello Tim and welcome to 'C'.

Placing a semicolon ( ; ) at the end of the compare section of the "if" statement will cause that line of code to simply resolve to TRUE or FALSE (a "1" or "0" ) and have no effect. Therefore, it will be optimized away by the compiler. The offending statement is pointed out in your code (below) in black font.

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); <== remove this " ; " //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
}

Note that for "while" statements, the semicolon is valid and denotes the end of the while loop.

Regards,

Frank

JimDon · ‎01-19-2008

Could you post all the code ?
And please just cut and paste it from the IDE with out the asm code.
perferable in and SRC section (see the buttons the one marked SRC)

thisobj · ‎01-19-2008

Tim,

It sounds like you may have Key_temp defined in another file or somewhere after it's use. Since this is a hardware-related variable, you most likely (or should have) defined it as a volatile char in your interrupt module. Key_temp's scope is then limited to that file which contains your key interrupt service. If you want to use it in other modules (files), those modules must use an extern statement, as in the following example:

extern volatile char Key_temp; // Key_temp defined elsewhere.

void (void )
{
if(Key_temp == 0x48)
{ //code in here }
}

In another file:

volatile char Key_temp;

kbd_svc( void )
{
Key_temp = {read from hardware source};
}

JimDon · ‎01-19-2008

Code:

    if(Key_temp==0x48)                 //Up - short—           //       if(Month_current==12){//         Month_current=1;//         Update_month();//       }If you comment out that code, then it won't compile!!!!
With out the second if block, then you do need a ';'
BUT
    if(Key_temp==0x48)                 //Up - short—          
        ;
will generate NO CODE because there is nothing to do!


 This will compile:    if(Key_temp==0x48)                 //Up - short–                  if(Month_current==12){         Month_current=1;         Update_month();       }
BETTER:
    if(Key_temp==0x48) {                 //Up - short–          
       if(Month_current==12){
         Month_current=1;
         Update_month();
       }
   }

Because:
    if(Key_temp==0x48) {                 //Up - short–          
//       if(Month_current==12){
//         Month_current=1;
//         Update_month();
//       }
   }
WILL compile (but still generate no code).

UtopiaTim · ‎01-21-2008

Hi Jim/all,

Came across another question in my debugging.

On the red arrow'd calls, is there a way to force them to be JMPs & not JSRs?

    if(Key_temp==0x48) {                 // Up - short (increment month)
       if(Month_current==12){           // if month = 12 (max)
         Month_current=1;                 // then make it a 1
         Update_month();                  // go update                                     <------ ??
       }

       Month_current++;                  // otherwise, increment month
       Update_month();                   // and go update & back to Do_month    <-------- ??
    }

Thanks,

Tim

JimDon · ‎01-21-2008

Well there is a goto in C but you can't really goto a function and find your way back, as goto can only be used with in a function.

You haven't told me enough for me to suggest an alternative.
However this may be what you want:

Code:

   if(Key_temp==0x48) {                // Up - short (increment month)                        if(Month_current==12)           //  if month = 12 (max)         Month_current=1;              //  then make it a 1        else                                 Month_current++;             // otherwise, increment month       Update_month();                 // and go update & back to Do_month        }

If you show me more of your code, perhaps I could help you restucure it.
You need to see new patterns of design not using jumps.
You might find this interesting: GOTO Considered harmful

This is from 1968.

Lundin · ‎01-21-2008

If JSR instructions are too slow, you should use "inline functions". The concept of inline functions is however not part of the classic C91 ISO standard, but was added in the C99 standard as well as C++.

So in classic C programming, there is no standard way to do this. By declaring the function static, the compiler may if you are lucky "inline" that code. This is compiler-dependant.

In Codewarrior, you could use the #pragma INLINE directive to force inlining of a particular function.

UtopiaTim · ‎01-21-2008

Hi Jim,

Here's the flow chart that I did (based upon assembly

language work). (maybe that's the problem!)

I made the decision triangles = boxes... easier to draw!

       | Do_month          |------------|
       |--------|<----------|update_month|<--|
   |--------| Y |           |------------|   |
   | KB=00? |---|                            |
   |--------|                                |
       |                                     |
       |                                     |
   |--------| Y |--------| |------------| |
   | KB=48? |----|month=12|--|make month=1|->|
   |--------|    | ??    | |------------| ^
       |         |--------|      |           |
       |                         |           |
       \/                    |---------|     |
   more key checks           |Inc Month|-----|
   that also go to           |---------|

Update_month

The bottom key check

that sees no valid key

will go directly back

to Do_month

One of the key checks

will exit this routine

and fall into the next

group of key checks. (after

updating an LCD display).

This pattern of decisions is duplicated for other

key presses, all going back to Do_month through

Update_month.

I'd like to do it right... putting an

asm{

jmp Update_month

}

probably isn't the 'c' way to do things!

Thanks,

Tim

JimDon · ‎01-21-2008

Alright, I think what you need is the continue statement:
This may not be exactly what you need, but this is what we do in "C" instead of jmp ing.

Code:

for(;;){  char kb = readkey();  if( 0 == kb )   continue;    // go back and read the key. (jmp back to top)  if( 48 == kb)  {      // do stuff      continue; // got back to the top  }  if(xx == kb) {     // do stuff for xx    continue; }}

Or you could use a switch statement

Code:

  switch (kb) {   case 48:    // do 48 stuff    break;  case xx:   // do xx stuff  break;  default: // 0 will end up here as there is nothing to do.    ;}

Message Edited by JimDon on 2008-01-21 11:33 AM

allawtterb · ‎01-21-2008

The problem with the inline pragma is that it will take alot more code space, especially if Update_month is called in a large number of places and the code for Update_month is long.

The continue statement JimDon posted will go back to the top the way you wanted in your flow chart but doesn't solve replacing the JSR's with JMP's. If Update_month is only being called by this function, which it seems like it must from the flow chart you were showing, there should be a solution but it might not be very nice looking C. In fact, it would be considered really poor looking C because of the use of the goto command. BUT, for the sake of "how can this be done", I believe it can be done as follows.

Take the code for Update_month and place it at the bottom of Do_month. If you are checking for a large number of different options then a switch and case statements would be the best way to find the press you are looking for.

Code:

void Do_month(void){START:   while (Key_temp == 0)  // Wait for key press      ;      switch (Key_temp)   {      case XX:..... // All your previous cases here      case 48:      // Case for incrementing the month by 1              if (Month_current == 12)   // If at the end of the year              {                 Month_current = 1;      // Set to the start of the year              }              else                                     {                Month_current++;         // Otherwise increment the month by 1              }               goto UPDATE_MONTH;         // Now update month        case XX:...  // the rest of your case statements here   }     goto START;     // If no matches go back to the start  UPDATE_MONTH:   // Insert your Update_month code here   goto START;}     // Any case that you want to exit Do_month must end with a "return;"

You could put a "while(TRUE)" at the top of the function thats scope includes the whole function, then replace "goto START;" with a "continue", but I don't see much difference. Once again, this is a very very bad way to code C and I would suggest just using the switch statement if there are more than 3 different possible key presses and doing a regular call to a seperate function called Update_month.

If you do go with if's for checking to see what key is pressed, use "if (key==XX)...else if (key==XX)" with the most common key occurance being the first "if" and the least towards the end.

UtopiaTim · ‎01-22-2008

Thanks Jim & allawtterb,

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).

The only thing I couldn't get was how to make the function exit from the function & go

to another function. (one of the keys should exit from the original function & go to another).

For time expediency, I took the easy way out... did an asm{.. jmp xxxx}

I am starting to get away from the old assembly code habits, but it's gonna take a while.

Thanks again to everyone.

Tim

JimDon · ‎01-22-2008

Tim,
Don't take this as a flame, but as some advice.
Are you going to learn "C" or not?

The way out of the for loop is the "break" statement.

Presumably you know the advantages of "C", and I do not want to start a "C" vs asm thread here, but 90% of the code in this domain is written in "C". If you were interviewing for a job, and they saw that you would not get the job. Sometimes there is a good reason for putting asm in "C" code - like enabling/disabling interrupts but for the most part is to be avoided as it becomes a portability issue.
I understand what you are going thru - I did it myself (like 30 years ago), but once you see how it works it is not that hard. At first it seems impossible not to use goto, but you can. Any code you write with gotos, can be written in "C" (or Java or C# or even (choke) VB) without them.

If you post some code, I or anyone else on this forum would be glad to show you.

I know you are pressed for time, but I have found that sometimes it is wise to spend the time to learn things, it saves more time in the long run. Using jmps is likely to eventually crash the stack, and then you will spend even more time figuring that out.

Lundin · ‎01-25-2008

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.

for(;;)
{
  char kb = readkey();

  if( 48 == kb)
  {
    /* do stuff */
  }
  else if(xx == kb)
  {
    /* do stuff for xx */
  }
  else
  {
    /* 0, do nothing */
  }
}

---

And off-topic, to defenders of VB: how did you do error handling in pre-VB.NET? You used "on error goto" since the language didn't have any other error support, ie you were forced to use poor programming practice by the language itself. So I don't think the choise of language is irrelevant to the structure of the code.

JimDon · ‎01-25-2008

Well, for one thing, there is s huge difference between continue and goto.

The target of a continue is one and only one place, so it is pretty easy to understand the code.
I don't like large if-else if blocks or switch statements either, but still you need such logic.
A properly used got is fine, like only to a common error exit where clean up is required.

So please provide a better alternative implementation method, and please can we have a link to the MISRA coding standards so that we might completely understand these ideas.

allawtterb · ‎01-25-2008

JimDon, I believe that the rules are only avaliable for purchase. It seems there has been some general discontent with the MISRA-C rules on Flow Control (goto, continue and break). Within the MISRA-C rules you are allowed deviations. Deviations can allow you to break a MISRA-C rule as long as you can defend the code that breaks the rule. If you have a question about a particular situation you can always post it on their forums:

http://www.misra-c2.com/forum/viewforum.php?f=53

UtopiaTim · ‎01-22-2008

Hi Jim, No flame taken.. you made the jump 30 years ago!

Maybe I need to look at this a bit differently. Since gotos & other jumps are verboten

in C, then maybe I should look at how it is structured.

Currently, I am reading a location in memory that is changed by an ISR (key_temp).

This portion of the program uses different keypresses to change the display of a LCD (and

eventually the data in the module).

I had been looking at it as a serial process, first check & change the months, when done

there, go to the next procedure of changing the days, hours, minutes, etc, etc.

Within each of these small chunks of program, there are ways to exit the code, for example

if the user just wants to leave & return back to the top of the program. The other is to jump

out of the date set, and go to another setting program called med_setup.

If I were to call each of these setting programs (month, day, year, hour, minute, am/pm) as

a function into themselves, each could return with a value that would say (wants to leave,

wants to do med_setup, go to next function, etc).

Within each function, I can do the proper updates, etc, by using the break; instruction &

no gotos - tnx!!.

But, once each function returns with a value that says what it wants to do, I am back into

the same problem of how do I go directly to a new function, without any kind of return back

to where it came from.

Tim

PeterHouse · ‎01-22-2008

JimDon,

No Flames Intended Here. Just defending VB.

It is easy to trash VB as the first version of BASIC did not lend themselves to structured programming very well although a programmer using structured techniques could still manage to write a structured program.

I have been writing in VB almost as long as C and in C almost as long as asm and have seen examples of C code which demonstrated how C could be used in an unstructured way. In any case, it is the programmer who decides if the program will be structured or not. The language chosen is irrelevant regarding the structure of the program.

UtopiaTim,

As you learn C you will most likely find better ways to structure and document your Assembler code and since you are commfortable with assembler now, it will make you a better C programmer in the long run.

Good Luck,

Peter House

UtopiaTim · ‎01-23-2008

Hi Peter,

Thanks for the comments.

At this time, I think that the project is going to be

a combination of both assembly and c.

There have been several times when I wanted to

drop the c entirely, but I have been learning quite

a lot, and overall it makes sense to migrate that

way.

Since this is my first c program (and a real project

with a deadline), it makes sense (to me anyway)

to do the best I can with the tools & knowledge that

I have (and am getting).

Anyhow, thanks to everyone for the tips... many of

them are being used too!!

Thanks,

Tim

C Question

C Question

General