0
int main(int argc, char *arv[])
{
   printf("Enter sequence of numbers seperated by comma\n");
   char input[200];
   char *valueStr = NULL;
   int total = 0;
   int values[100];
   int ctr = 0, i = 0;
   if (fgets(input, sizeof(input), stdin) != NULL)
   {  // split using comma 
      printf("printing %s", input);
      valueStr = strtok(input, ",");
      printf("printing %s", valueStr);
      while (valueStr != NULL && sscanf(valueStr, "%d", values[ctr++]) != EOF);
      {
         printf("Got value %d\n", values[ctr - 1]);
         valueStr = strtok(NULL, ",");
      }
      for (i = 0; i < ctr; i++)
         total +=values[i];
      if (total == 0)
         printf("Equals to 0\n");
      else
         printf("Doesn't equal to 0\n");
   }
}

Seems very simple to me... What am I doing wrong? Currently the output is the following:

bash-4.2$ ./subset 
Enter sequence of numbers seperated by comma
-5,-3,-1,2,4,6
printing -5,-3,-1,2,4,6
Segmentation fault (core dumped)

It never gets passed strtok() and I don't see why.. I'm not passing a string literal into the function!

user1410668
  • 245
  • 3
  • 6
  • 16
  • 5
    `while (valueStr != NULL && sscanf(valueStr, "%d", values[ctr++]) != EOF);` : Remove last `;` – BLUEPIXY May 03 '17 at 21:08
  • 1
    What does the debugger show you? If you're not using it then you should. You'd have solved it yourself in minutes. – hookenz May 03 '17 at 21:11
  • 4
    `sscanf(valueStr, "%d", values[ctr++])` --> `sscanf(valueStr, "%d", &values[ctr++])` – BLUEPIXY May 03 '17 at 21:12
  • 1
    There are multiple things wrong. Read these comments carefully. – WhozCraig May 03 '17 at 21:15
  • The big mistake was forgetting the & in the scanf. Anyway it works now. Thanks everyone. – user1410668 May 03 '17 at 21:15
  • 4
    Note that `sscanf(valueStr, "%d", values[ctr++]) != EOF` will increment `ctr` even if there is a bad read. Anyway `sscanf` result should be tested with `1` not `EOF`. – Weather Vane May 03 '17 at 21:16
  • Note [Using `sscanf()` in loops](http://stackoverflow.com/questions/3975236/how-to-use-sscanf-in-loops). – Jonathan Leffler May 03 '17 at 22:31
  • ... and the need for a `-1` in `printf("Got value %d\n", values[ctr - 1]);` reinforces my previous comment. Increment `ctr` when all is good. – Weather Vane May 03 '17 at 22:50
  • When compiling, always enable the warnings, then fix those warnings (for `gcc`, at a minimum use: `-Wall -Wextra -pedantic` I also use `-Wconversion -std=gnu11` ) This will expose several problems with the posted code. – user3629249 May 04 '17 at 17:21
  • The posted code does not compile! it is missing the needed `#include` statements. Do you expect us to guess as to what header files your code actually included? – user3629249 May 04 '17 at 17:22
  • after adding the missing `#include` statements for `stdio.h` and `string.h` and the parameter to `sscanf()` is corrected, there is still the problem that the parameters to the function: `main()` are not being used. When the parameters are not being used, use the signature: `int main( void )` – user3629249 May 04 '17 at 17:26
  • regarding this expression: `sscanf(valueStr, "%d", &values[ctr++]) != EOF);` the EOF is not the correct comparison. Rather use: `sscanf(valueStr, "%d", &values[ctr++]) == 1);` – user3629249 May 04 '17 at 17:31
  • the `while()` statement should also be checking the value of `ctr` to assure that the array `values[]` is not being overflowed. Suggest: `while (valueStr != NULL && ctr < 100 && sscanf(valueStr, "%d", &values[ctr++]) == 1);` – user3629249 May 04 '17 at 17:35
  • The posted code contains some 'magic' numbers. 'magic' numbers are numbers with no basis. I.E. 100. 'magic' numbers make the code much more difficult to understand, debug, etc. Suggest using a `enum` statement or `#define` statement to give those numbers meaningful names, then use those meaningful names throughout the code. – user3629249 May 04 '17 at 17:37

0 Answers0