1

I am trying to make a C string calculator. This means that I have a string with numbers and a delimiter in the middle. the delimiter can be of any size as long as it isn't a number. Also, if the value of a specific number is between 1001 and 1111 it cannot be used and will be ignored (set to zero). The difficulty of this assignment is to make the delimiting/splitting string part without strtok. I am getting all sorts of wrong outputs and I have no idea what I am doing wrong, yet I feel like I am oversighting something or doing something incredibly stupid. The code also freezes my unit tests, so I cant even test what is going wrong.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
#include <stdbool.h>

void ValidateInput(int *x) {
    if (*x >= 1001 && *x <= 1111) { 
        x = 0;
    }
}

int test(char *numbers, int numbers_length, int *result) {
    char cnum1[numbers_length];
    char cnum2[numbers_length];

    strcpy(cnum1, "");
    strcpy(cnum2, "");

    bool x = false;
    size_t i = 0;

    while (numbers[i] != '\0') {
        char c = numbers[i];
        if (isdigit(c)) {
            if (x) {
                strcat(cnum2, &c);
            }
            if (!x) {
                strcat(cnum1, &c);
            }
        }
        if (!isdigit(c)) {
            x = true;
        }
        i++;
    }

    int num1;
    num1 = atoi(cnum1);
    int num2 = atoi(cnum2);

    ValidateInput(&num1);
    ValidateInput(&num2);

    printf("%d\n", num1);
    printf("%d\n", num2);
    
    return num1 + num2;
}

int main() {
    int sum;
    char numbers[] = "100,10";
    int length = strlen(numbers);
    test(numbers, length, &sum);
    return -1;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 1
    Have you tried running your code line by line in a debugger while monitoring the values of all variables, in order to determine at which point your program stops behaving as intended? If you did not try this, then you probably want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) You may also want to read this: [How to debug small programs?](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Andreas Wenzel May 23 '21 at 19:45
  • 'strcat(cnum2, &c);' strcat() requires NUL-terminated char arrays for both arguments - passing the address of one char is UB, (unless the char is a NUL). – Martin James May 23 '21 at 19:46
  • Go through all your code and check all str* library calls are supplied with valid arguments and that all arrays are long enough to accommodate any 'strings' copied in, including NUL-terminators. – Martin James May 23 '21 at 19:48
  • Right off the bat I feel it's important to point out that in `ValidateInput`, you're checking the value of the pointer (which is fine), and then if it's invalid, setting the *address of the pointer to 0* (which is NOT fine). That's going to cause a segfault if you try to access `x` after that. – Nick Reed May 23 '21 at 19:55
  • How many delimiters in the source string? And how are they supposed to be split across strings? If only 1 delimiter, I assume everything after that would go into the second string? It might help to post an example or two of how the data is supposed to be modified in your question. – Michael Dorgan May 23 '21 at 20:07
  • Also note that if the number passed in is too big, you will overflow the size of an integer. That might be out of scope for your assignment though. – Michael Dorgan May 23 '21 at 20:08

2 Answers2

1

There are some problems in the code:

  • x = 0 in ValidateInput() has no effect outside the function. You should write *x = 0;

  • strcat(cnum2, &c); is incorrect: &c is not a proper C string. You should either convert the number on the fly or append the character using an index.

  • test() does not need the length of the string, it can test for a null terminator.

  • if (!x) and if (!isdigit(c)) are redundant, just use an else clause.

  • test() returns the sum, it should not take the address of the sum as an argument and its return value should be stored into sum in the main() function.

Here is a modified version:

#include <ctype.h>
#include <stdbool.h>
#include <stdio.h>

void ValidateInput(int *x) {
    if (*x >= 1001 && *x <= 1111) { 
        *x = 0;
    }
}

int test(char *numbers) {
    int num1 = 0;
    int num2 = 0;
    bool x = false;

    for (size_t i = 0; numbers[i] != '\0'; i++) {
        unsigned char c = numbers[i];
        if (isdigit(c)) {
            if (x) {
                num2 = num2 * 10 + c - '0';
            } else {
                num1 = num1 * 10 + c - '0';
            }
        } else {
            x = true;
        }
    }

    ValidateInput(&num1);
    ValidateInput(&num2);

    printf("%d\n", num1);
    printf("%d\n", num2);
    
    return num1 + num2;
}

int main() {
    char numbers[] = "100,10";
    int sum = test(numbers);
    printf("sum=%d\n", sum);
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Should add his `!isdigit()` is not needed and should just be an else. – Michael Dorgan May 23 '21 at 20:18
  • Like your atoi replacement. Should make that an interview question for someone in the future :) - Feel like an array for the num1/num2 might simplify the if logic a bit as well. (x becomes index.) – Michael Dorgan May 23 '21 at 20:25
  • @chqrlie , you are a litteral life saver. I cannot thank you enough! your code works its wonders. I do have a question however. What does this line of code do: num2 = num2 * 10 + c - '0'. Why are you multiplying? Do you have a source so i can read up on this, or would you mind explaining this to me? –  May 23 '21 at 20:26
  • That is `atoi` in short form. Walk through it on paper and watch how it works :) – Michael Dorgan May 23 '21 at 20:27
  • @MichaelDorgan thank you, i appreciate the help :) –  May 23 '21 at 20:31
  • @CodeBoy: `num2 = num2 * 10 + c - '0'` works incrementally: `num2` is the value read so far. `c` is the next character, if it is a digit, we multiply the number read so far by 10 and add the digit value of `c`. The characters for the digits `0` through `9` are consecutive so `c - '0'` is the digit value corresponding to the character `c`. – chqrlie May 23 '21 at 20:52
-1

You forgot Nullbytes \0

char numbers[] = "100,10\0";
char cnum1[numbers_length]; //cnum1[3] = '\0'; for value 100 before  atoi(cnum1);
char cnum2[numbers_length]; //cnum2[2] = '\0'; for value 10  before  atoi(cnum2);

And a printf by the own function call

printf("%d\n",test(numbers, length, &sum));

to see the result of add. ^^

You can also store the result in your result variable is a ref.

//Hint: int test(..., int* result)
result = num1 + num2;
//and in the main
printf("%d\n",sum);

if you want the return use otherwise.

return num1 + num2;

Hint:

int test(char* numbers, int numbers_length, int* result)
{
   char cnum1[numbers_length];
   char cnum2[numbers_length];
   char* ptr1 = &cnum1[0];
   char* ptr2 = &cnum2[0];
...
   if(x)
   {
     *ptr2 = c;
     ptr2++;
   }

   if(!x)
   {
     *ptr1 = c;
     ptr1++;
   }
   ....
   *ptr1 = '\0';
   int num1 = atoi(cnum1);
   *ptr2 = '\0';
   int num2 = atoi(cnum2);
  • 1
    Pretty certain that the compiler will add the \0 for you if you place it in quotes like this. See https://godbolt.org/z/EGzh8occv and notice that that strlen is correct in the assembler. – Michael Dorgan May 23 '21 at 20:00
  • 1
    That said, he did forgot the NUL byte when he switching from cnum1 to cnum2 and a final NUL on cnum2... – Michael Dorgan May 23 '21 at 20:10
  • 1
    Except that you also forgot to NUL terminate your strings on buffer switch and final write. You might get away with it on a debug build where `cnum1` and `cnum2` are set to 0 for you, but in release, that will fail. Consider using an `else` for your `if(!x)` as well - simpler logic. Finally, if you are going to post an answer as a solution, make sure in compiles completely. I personally find it helps to link a godbolt or other online compiler result to verify my code. – Michael Dorgan May 23 '21 at 20:17
  • Now it is work correctly. The nullbytes are placed. sorry, I forgot it myself. ;P – MitnickCodeHelper May 23 '21 at 20:20
  • @MichaelDorgan I do not want to replace to much of original code. It must be readable for the asking person or not? – MitnickCodeHelper May 23 '21 at 20:27
  • 1
    Sure - I have no problems with code pieces that demonstrate a change, but don't call it a "solution" if it is not a solution. Look at the upvoted answer for an example of this. And thank you for updating your answer and accepting some advice. :) I am actually considering deleting my answer because the top answer doubles my advice... – Michael Dorgan May 23 '21 at 20:30