C strcat inserts garbage into string

Question

My method reads an input text of vectors with the following format:

57.0000,-7.4703,-0.3561
81.0000,-4.6478,7.9474
69.0000,-8.3768,0.4391
18.0000,-4.9377,9.9903
62.0000,-5.8751,-6.6054
...

My attempt to read each vector and insert it to an array is as follows:

FILE *file;
int n = 1, dim, i=0;
char* str;
double ret;
double* X;
int c;
int com=0;
assert(argc==2 && "argc != 2");
file = fopen(argv[1], "r");
assert(file && "file is empty");
for(c = getc(file); c!= EOF; c = getc(file)){
   if(c == '\n'){
        n++;
   }
   else if(c==','){
       com++;
   }
}
dim = com/n +1;
char* str;
double ret;
double* X;
X = (double *)calloc(n*n, sizeof(double));
assert(X); 
str = (char *)calloc(100, sizeof(char));
assert(str); 
for(c = getc(file); c!= EOF; c = getc(file)){
     if(c!=',' && c!= '\n'){
       strcat(str, &c);      
     }
     else{
     ret = strtod(str, NULL);
     X[i] = ret;
     i++;
     memset(str, 0, 100 * sizeof(char)); 
     }
}

The problem is that when it gets to the last vector at each line, it reads each char and concatenates it with extra garbage into str. Any ideas how to solve this?

Show how/where you open the file. Where is `c` defined? ( [mcve] would be good here.) — ryyker, Sep 08 '21 at 16:16
The type of `c` is not shown (before the edit), but it is certain not to be suitable for passing to `strcat` because you have `c = getc(file)` so it should be `int` type. — Weather Vane, Sep 08 '21 at 16:17
`strcat(str, &c);` is invalid as `&c` does not point to a _string_. — chux - Reinstate Monica, Sep 08 '21 at 16:22
I have added the whole code of this section. I define c as char and not as int because there are signs which I need ('-', ',') etc. how do I do this correctly? — Kahalon, Sep 08 '21 at 16:22
Remember that `'-'` is type `int` not `char`. You can't test for `EOF` without using `int` which is what the library function `getc` returns, *not* `char`. — Weather Vane, Sep 08 '21 at 16:23
Thank you everyone, I have updated c to an int type but the issue still remains — Kahalon, Sep 08 '21 at 16:24
Read the comment by @chux then. You can't `strcat` a single character, you need a string. — CiaPan, Sep 08 '21 at 16:24
`char smallstr[2] = {0}; smallstr[0] = c; strcat(str, smallstr);` — Weather Vane, Sep 08 '21 at 16:25
The 2nd `c = getc(file)` loop certainly returns `EOF`. Perhaps `rewind(file)` before 2nd time through the file. — chux - Reinstate Monica, Sep 08 '21 at 16:36

Olaf Dietsche · Answer 1 · 2021-09-08T16:32:18.610

strcat expects a NUL-terminated string (array of char) as its second argument, but c is defined as a single character, not an array of char.

To fix this, you can maintain an index into str

int c;
int j = 0;
for (c = getc(file); c!= EOF; c = getc(file)) {
    if (c != ',' && c != '\n') {
        str[j++] = c;
        str[j] = 0; // keep string NUL-terminated
    } else {
        ret = strtod(str, NULL);
        X[i] = ret;
        i++;
        // reset j
        j = 0;
    }
}

score 3 · Answer 2 · edited Sep 08 '21 at 16:54

3

As c is char the following is invalid.

c = getc(file); c!= EOF;

and

strcat(str, &c);

Both are Undefined behaviours. To sort the first out declare c as int

The second problem:

//You need to create a null char terminated string to use as 
//second parameters of the srtcat. For example, you can define 
//a compound literal - char array containing two elements: c & 
//terminating null character
strcat(str,(char []){c,0});

edited Sep 08 '21 at 16:54

alinsoar

15,386
4
57
74

answered Sep 08 '21 at 16:32

0___________

60,014
4
34
74

2

`c = getc(file); c!= EOF;` is not UB, yet still poor code. Nifty to use a _compound literal_. – chux - Reinstate Monica Sep 08 '21 at 16:35
I did not want to change OPs code too much – 0___________ Sep 08 '21 at 16:38
1

Fair enough to limit changes to OP code, yet answer's explanation remains amiss. – chux - Reinstate Monica Sep 08 '21 at 16:40

C strcat inserts garbage into string

2 Answers2