Quick question regarding this issue, Why doesnt it print out the second value(converted second value) on the string?

Question

Quick question, What have I done wrong here. The purpose of this code is to get the input into a string, the input being "12 34", with a space in between the "12" and "32" and to convert and print the two separate numbers from an integer variable known as number. Why doesn't the second call to the function copyTemp, not produce the value 34?. I have an index_counter variable which keeps track of the string index and its meant to skip the 'space' character?? what have i done wrong?

thanks.

#include <stdio.h>
#include <string.h>
int index_counter = 0;
int number;
void copyTemp(char *expr,char *temp);

int main(){
 char exprstn[80]; //as global?
 char tempstr[80];

 gets(exprstn);
 copyTemp(exprstn,tempstr);
 printf("Expression: %s\n",exprstn);
 printf("Temporary: %s\n",tempstr);
 printf("number is: %d\n",number);
 copyTemp(exprstn,tempstr);      //second call produces same output shouldnt it now produce 34 in the variable number?
 printf("Expression: %s\n",exprstn);
 printf("Temporary: %s\n",tempstr);
 printf("number is: %d\n",number);

 return 0;
}
void copyTemp(char *expr,char *temp){
 int i;
 for(i = index_counter; expr[i] != '\0'; i++){
  if (expr[i] == '0'){
   temp[i] = expr[i];
  }
  if (expr[i] == '1'){
   temp[i] = expr[i];
  }
  if (expr[i] == '2'){
   temp[i] = expr[i];
  }
  if (expr[i] == '3'){
   temp[i] = expr[i];
  }
  if (expr[i] == '4'){
   temp[i] = expr[i];
  }
  if (expr[i] == '5'){
   temp[i] = expr[i];
  }
  if (expr[i] == '6'){
   temp[i] = expr[i];
  }
  if (expr[i] == '7'){
   temp[i] = expr[i];
  }
  if (expr[i] == '8'){
   temp[i] = expr[i];
  }
  if (expr[i] == '9'){
   temp[i] = expr[i];
  }
  if (expr[i] == ' '){ 
   temp[i] = '\0';
   sscanf(temp,"%d",&number); 
   index_counter = i+1; //skips?
  }
 }
 // is this included here? temp[i] = '\0'; 
}

Answer 1

There are a few problems in your program:

• You are using the same index into expr and temp arrays. This works for the first time since both will be 0 to start with but when you want to process the 2nd number, you need to reset the index into the temp array back to 0. Clearly this cannot be done using a single index. You'll have to use two indices, i and j.
• By the time you complete the processing of the 2nd number ( 34 in "12 34") you'll reach the end of the string and hence the sscanf never gets run on the second occasion ( in general for the last occasion). So after the for loop you need another sscanf to extract the last number. Also you should return from the function once you've extracted the number from the string and incremented i.
• You should avoid using gets() and use fgets() instead because of security reasons.
• You can combine the multiple test for the digits into a single test as shown:

Something like this.

void copyTemp(char *expr,char *temp){
    int i;
    int j = 0;
    for(i = index_counter; expr[i] != '\0'; i++){

        if (expr[i] >= '0' && expr[i]<='9'){
            temp[j++] = expr[i]; // copy the digit into temp..increment j.
        }    
        else if (expr[i] == ' '){ // space found..time to extract number.
            temp[j] = '\0'; // terminate the temp.
            sscanf(temp,"%d",&number); // extract.
            index_counter = i+1; // skip the space.
                    return; // done converting...return..must not continue.
        }
    }
    // have reached the end of the input string..and still need to extract a 
    // the last number from temp string.
    temp[j] = '\0';
    sscanf(temp,"%d",&number);
}

After these changes it works as expected:

$ gcc b.c 2> /dev/null && ./a.out
12 34
Expression: 12 34
Temporary: 12
number is: 12
Expression: 12 34
Temporary: 34
number is: 34

Your approach is very fragile...if a user gives multiple spaces between the input numbers..your program will fail.

Answer 2

The main problem is that copyTemp writes to temp[i], but each call to copyTemp initializes i to index_counter, not to 0. This means that each call to copyTemp appends to the existing temp buffer instead of overwriting the old contents, and sscanf thus always re-reads the same string. You need to use separate indices to keep track of where to read from the input buffer and where to write to the output buffer.

Additional problems: * Never** use ggets. Ever. Use fgets instead. * You duplicate a lot of code in copyTemp. You instead could do:

if (expr[i] == '0' || expr[i] == '1' || ...)

or better:

if (isdigit(expr[i]))

• copyTemp should take some precautions to not overflow its destination buffer. (Note that copyTemp shouldn't even need to take a destination buffer as an argument.)

• You should avoid using global variables. It'd be better for copyTemp to take an argument specifying where to start reading from the input string and if it returned the index where it left off.