casting, pointers, confusion--help

don89c
2007-10-11
2013-03-12
  • don89c
    don89c
    2007-10-11

    All,

    A program that I've inherited is written in C and running on an Atmel AT89C51ED2 microcontroller.  It is compiled using the free download Keil C compiler.  I'm having trouble determining what the following if statements accomplish.  My guess is that the intent is to set the variable g_ul_clock_count to zero (0) whenever all four bytes in this variable are 0xFF.  Otherwise, the value of the variable is incremented by one.

    unsigned long data g_ul_clock_count=0;  //this is a global variable

    //timer 0 super clock count interrupt routine
    void timer_0_interrupt(void) interrupt 1 using 3
    {
    TF0=0; // reset interrupt flag
    if(!(~(((unsigned char data *)(&g_ul_clock_count))[3])))
      if(!(~(((unsigned char data *)(&g_ul_clock_count))[2])))
       if(!(~(((unsigned char data *)(&g_ul_clock_count))[1])))
        if(!(~(((unsigned char data *)(&g_ul_clock_count))[0]))) g_ul_clock_count=0;
    g_ul_clock_count++;
    }

    Am I correct in my interpretation of this code?

    Thanks!

     
    • Hi,

      this is strange code... Starting from top:

      1) g_ul_clock_count=0;  Unless the startup code is touched there
                  should be no need to initialize to zero

      2) using 3  there should be no need to use using 3. Why spend
                  a complete register bank for just one or two registers?

      3) TF0=0;   hardware does it. No need for software

      4) if(..)   these would be completely redundant if there also would
                  be an "else": (Why check for 0xFFFFffff and then set
                  to zero? (If it is 0xFFFFffff the increment would also
                  have resulted in zero).
                  Without the "else" this seems to be intended to be
                  a counter which skips zero once it is running...
                  (yet the ~ operator on a char or bit variable is problematic)

      If you want to compare Keil with SDCC generated code, you could
      insert the missing casts to (unsigned char) like this:

      if(!((unsigned char)~(((unsigned char data *)(&g_ul_clock_count))[3])))
      if(!((unsigned char)~(((unsigned char data *)(&g_ul_clock_count))[2])))
      if(!((unsigned char)~(((unsigned char data *)(&g_ul_clock_count))[1])))
      if(!((unsigned char)~(((unsigned char data *)(&g_ul_clock_count))[0]))) g_ul_clock_count=0;

      If the zero skipping counter is really intended you could also do:

      //timer 0 super clock count interrupt routine
      void timer_0_interrupt__(void) interrupt 1
      {
          g_ul_clock_count++;
          if(!g_ul_clock_count)
              g_ul_clock_count++;
      }

      Greetings,
      Frieder

       
      • Yuck. The nested ifs seem to test the bytes in a long word for zero, which the compiler can sort out. I would also agree that the code inside the if skips the zero value.

        When you come across code like this it can help readability if you use braces and indentation as follows:
        if(!(~(((unsigned char data *)(&g_ul_clock_count))[3])))
        {
            if(!(~(((unsigned char data *)(&g_ul_clock_count))[2])))
            {
                if(!(~(((unsigned char data *)(&g_ul_clock_count))[1])))
                {
                    if(!(~(((unsigned char data *)(&g_ul_clock_count))[0])))
                    {
                        g_ul_clock_count=0;
                    }
                }
            }
        }
        g_ul_clock_count++;

         
    • don89c
      don89c
      2007-10-11

      Frieder,

      Thanks!

      Re:  1), 2) and 3) -- I understand.

      Re: 4)

      As an example

      if(!(~(((unsigned char data *)(&g_ul_clock_count))[3])))

      It appears that (unsigned char data *)(&g_ul_clock_count))[3]) would select byte[3] of the long integer (e.g., 0xFF).

      It appears that the intent of the ~ is to take the one's complement of byte[3], which yields 0x00 if the cast (unsigned char) is put in from of the ~ as you suggested for SDCC.

      Finally, the ! would seem to result in the next nested if being executed only if the original byte were 0xFF.

      Where did I go wrong?

      P.S.  Thanks for the strange code comment.  I'm a newbie, but it seemed strange even to me.

       
      • > Re: 4)
        > if(!(~(((unsigned char data *)(&g_ul_clock_count))[3]))) 
        > It appears that (unsigned char data *)(&g_ul_clock_count))[3]) would select byte[3] of the long integer (e.g., 0xFF).

        Yes.

        > It appears that the intent of the ~ is to take the one's complement of byte[3],

        Yes.

        > which yields 0x00 if the cast (unsigned char) is put in from of the ~ as you suggested for SDCC.

        Yes.

        > Finally, the ! would seem to result in the next nested if being executed only if the original byte were 0xFF.

        No. This might be something unexpected (at least it was to me).

        The (unsigned char) is being extended to integer first (as required)
        and then inverted (similar problem as when applying ~ to a bit variable
        as shortly mentioned in manual section 1.4 or 6)

        Unfortunately SDCC does not warn about this (and also evaluates at run-time),
        but the high byte will be not zero and so the '!' is always true:

        ;       clock.c:8: if(!(~(((unsigned char data *)(&g_ul_clock_count))[3])))
                mov     r2,(_g_ul_clock_count + 0x0003)
                mov     r3,#0x00
                mov     a,r2
                cpl     a
                mov     r2,a
                mov     a,r3
                cpl     a
                mov     r3,a
                orl     a,r2
                jnz     00108$

        (r3 being #0xff when or'ed... so label 00108$ will always be jumped to)

        > P.S. Thanks for the strange code comment. I'm a newbie, but it seemed strange even to me.

        I don't believe you're a newbie. The title of your post, your posting style and
        the information you give tells different:)

         
    • don89c
      don89c
      2007-10-15

      Frieder and Oliver:

      Thanks for the feedback.  Your help is greatly appreciated!

      Gary