This code compiles cleanly and runs purely under valgrind .
#include <stdlib.h> #include <string.h> #include <stdio.h> static void svnViewStatus(void) { FILE *file; int i, j, k, q=0, ok; char mystring[100000], *vect[100000], *var, last[12], first[12]; file = fopen("db_svnViewStatus.txt", "r"); if (file == NULL) { printf("error"); return; } for (i = 1; fgets(mystring, sizeof(mystring), file) != 0; i++) { if (i == 1) /* Skip heading */ continue; char *space = mystring; for (j = 1; (var = strtok(space, ";")) != NULL; space = NULL, j++) { if (j == 3) { if (i == 2) { strcpy(first, var); } strcpy(last, var); } if (j == 2) { if (q != 0) { ok = 1; for (k = 0; k<q; k += 2) { if (strcmp(vect[k], var) == 0) { (*vect[k+1])++; ok=0; } } if (ok == 1) { vect[q] = malloc(strlen(var)+1); strcpy(vect[q], var); vect[q+1] = malloc(sizeof(int)); *vect[q+1] = 1; q += 2; } } else { vect[q] = malloc(strlen(var)+1); strcpy(vect[q], var); vect[q+1] = malloc(sizeof(int)); *vect[q+1] = 1; q += 2; } } } } fclose(file); printf("nr: %d \n", i-2); printf("first: %s \n", first); printf("last: %s \n", last); for (i=0; i<q; i = i+2) { printf("User %s: %d \n", vect[i], *vect[i+1]); } for (i=0; i<q; i=i+1) { free(vect[i]); } } int main(void) { svnViewStatus(); return 0; }
It has several levels with less indentation than the original. The code on the right side of the screen most often indicates problems. As noted in the comments, the main problem was the expression, which was intended to increment an integer using a pointer, but actually increase the pointer, not the integer. There are a couple of for loops that replace the while loops, so loop control is at the top of the loop. It is often easier to control the loop this way.
I'm still not happy with the code. You allocate int and treat char * as int * , which on a 64-bit machine means that you have an 8-byte pointer pointing to 4 bytes of data (and two calls to malloc() ). If you used a structure like:
struct data { char *string; int count; }; struct data vect[5000];
In this case, you only select (duplicate) the line in the next element of the structure. It would be more compact and there would be less risk of error. (You write vect[i].count++; and it will do what you need, without fuss, without garbage.) And you will not need to bother with q += 2; (or q = q + 2; ).
This code is better using structure. It also checks the memory allocation and ensures that the names copied to first and last do not overflow (and end with zero). It still does not check the bounds of the vect array to ensure that it does not overwrite the end. If an error occurred while reading the file, a memory leak occurs; cleaning will require some caution (and functions for this).
#include <stdlib.h> #include <string.h> #include <stdio.h> struct data { char *string; int count; }; static void svnViewStatus(void) { FILE *file; int i, q = 0; char mystring[100000], last[12], first[12]; struct data vect[50000]; file = fopen("db_svnViewStatus.txt", "r"); if (file == NULL) { printf("file open error\n"); return; } for (i = 1; fgets(mystring, sizeof(mystring), file) != 0; i++) { if (i == 1) /* Skip heading */ continue; char *space = mystring; char *var; for (int j = 1; (var = strtok(space, ";")) != NULL; space = NULL, j++) { if (j == 3) { if (i == 2) { strncpy(first, var, sizeof(first)-1); first[sizeof(first)-1] = '\0'; } strncpy(last, var, sizeof(last)-1); last[sizeof(last)-1] = '\0'; } if (j == 2) { int found = 0; for (int k = 0; k < q; k++) { if (strcmp(vect[k].string, var) == 0) { vect[k].count++; found = 1; } } if (found == 0) { vect[q].string = strdup(var); if (vect[q].string == 0) { printf("Memory allocation error\n"); return; } vect[q].count = 1; q++; } } } } fclose(file); printf("nr: %d\n", i-1); printf("first: %s\n", first); printf("last: %s\n", last); for (i = 0; i < q; i++) { printf("User %s: %d\n", vect[i].string, vect[i].count); } for (i = 0; i < q; i++) { free(vect[i].string); } } int main(void) { svnViewStatus(); return 0; }