Another C question

Question

I have a piece of code shown below

      #include <stdio.h>
      #include <stdlib.h>
      void Advance_String(char [2],int );
      int Atoi_val;
      int Count_22;
      int Is_Milestone(char [2],int P2);
      char String[2] = "0";
      main()
      {
         while(1)
         {

           if(Is_Milestone(String,21)==1)
           {
                 if(atoi(String)==22)
                 {
                     Count_22 = Count_22 + 1;
                 }
           }
           Atoi_val = atoi(String);
         Advance_String(S,Atoi_val); 
         }
       }
      int Is_Milestone(char P1[2],int P2)
      {
             int BoolInit;
             char *Ptr = P1;

             int value = atoi(Ptr);
             BoolInit = (value > P2);
             return BoolInit;
      }
     void Advance_String(char P1[2],int Value)
     {

             if(Value!=7)
             {
               P1[1] = P1[1]+1;
             }
             else
             {
               P1[1] = '0';
                  P1[0] = P1[0]+1 ;
             }
     }

Now my problem is Count_22 never increments as the char increments never achieves the value 21 or above.Could anyone please tell me the reason for this unexpected behaviour?My question here is to find the value of Count_22.Is there any problem with the code?

Thanks and regards,

Maddy

Answer 1

Your code is probably one of the worst pieces of C code i've ever seen (no offense, everybody has to learn sometime).

It has syntax errors (maybe copy/paste problem), logical problems, meaningless obfuscation, bad practices (globals), buffer overflow (atoi used on a char where there is no place to store the terminating zero byte), uninitialized values (Count_22), surprising naming convention (mixed CamelCase and underscore, variables and functions beginning with capital letter), infinite loop, no header and I forget some.

More, if you want anyone to help you debug this code, you should at list say what it is supposed to do...

To answer to the original question: why Count_22 is never incremented ?

Because Is_Milestone is always false (with or without @Jay change). Is_Milestone intend seems to be to compare the decimal value of the string "22" with the integer 21 (or 1, boolean result of 21 == 1) depending on the version).

It's logical because of Advance_String behavior. both because String has bad initial value (should probably be char String[3] = "00";) and because of the Value != 7 test. I guess what you wanted was comparing the digit with 7, but atoi works with a full string. Another minor change to achieve that Atoi_val = atoi(String+1); in the body of your loop. Then again you won't see much as the loop never stop and never print anything.

If it is a first attempt at an exercice given by some teacher (something like "programming a two digit counter in base 7" or similar). You should consider not using atoi at all and converting characters digit to value using something like:

digit_value = char_value - '0';

example:

char seven_as_char = '7';
int seven_as_int = seven_as_char - '0';

If you can explain what you are really trying to do, we may be able to show you some simple sample code, instead of the horror you are trying to debug.

EDIT

It is really more simple with original code...

After reading the Ada source, I can confirm it is indeed an Ascii based octal counter. The original code is allready of poor quality, and that explains part of the bad quality of the resulting C code.

A possible direct port could be as following (but still need a serious cleanup to look like native C code... and is quite dumb anyway as it prints a constant):

#include <stdio.h>
#include <stdlib.h>

void Advance_String(char * P1)
{
     if((P1[1]-'0') != 7){
         P1[1]++;
     }
     else{
         P1[1] = '0';
         P1[0]++ ;
     }
}


int Is_Milestone(char * P1, int P2)
{
    return (atoi(P1) > P2);
}

main()
{
    int Count_11 = 0;
    int Count_22 = 0;
    int Count_33 = 0;
    int Count_44 = 0;
    char S[3] = "00";

    int cont = 1;

    while(cont)
    {
        if(Is_Milestone(S, 10)){
            if(atoi(S) == 11){
                Count_11 = Count_11 + 1;
            }
            if(Is_Milestone(S, 21)){
                if(atoi(S) == 22){
                    Count_22 = Count_22 + 1;
                }
                if(Is_Milestone(S, 32)){
                    if(atoi(S) == 33){
                        Count_33 = Count_33 + 1;
                    }
                    if(Is_Milestone(S, 43)){
                        if(atoi(S) == 44){
                            Count_44 = Count_44 + 1;
                        }
                        if (atoi(S) == 77){
                            cont = 0;
                        }
                    }
                }
            }
        }
        Advance_String(S);
    }
    printf("result = %d\n", Count_11 + Count_22 + Count_33 + Count_44);
}

Answer 2

This statement

if(Is_Milestone(S,21==1) // Braces are not matching. If statement is not having the closing brace. Compilation error should be given.

should be

if(Is_Milestone(S,21)==1)

I guess.

Also, the code you have posted doesn't seem to be correct. It will surely give compilation errors. You have declared Count22, but are using Count_22.

Please check.