Arrays of pointers to strings - C

Question

I have a problem that I really can't understand. I'm a newbie C programmer, and I have a program that roughly goes like this:

void filetostr(FILE *, char *s[]);
void strtofile(char *s[], FILE *);
void XORstr(char *, int);
void XORtext(char *s[], int);
void printext(char *s[]);

int main(int args, char *argv[]) {

    char *s[MAXLENGTH];
    char ln[MAXLENGTH];

    FILE *input, *xorred, *rexorred, *out;

    input = fopen("input.txt", "r");
    filetostr(input, s);
    fclose(input);

    printext(s);

    XORtext(s, KEY);        
}

void filetostr(FILE *fp, char *s[]) {
    char ln[MAXLENGTH];
    char *p;
    int i = 0;

    while (fgets(ln, MAXLINE, fp)) {
        p = (char *) malloc(strlen(ln) * sizeof(char));
        strcpy(p, ln);
        s[i++] = p;
    }
}

void printext(char *s[]) {
    while (*s) {
        printf("%s", *s);
        s++;
    }
}

void XORstr(char *s, int key) {
    int c;
    while (c = *s)
        *s++ = key ^ c;
}

void XORtext(char *txt[], int key) {
    while (*txt) {
        XORstr(*txt, key);
        txt++;
    }
}

And I have two two problems:

• first, when I build the array of pointers to strings with filetostr, I get it to work but two lines in the middle of the text are repeated (there are two references to them in the array, so with printext they get printed twice). How is that possible? Is there a wrong call to malloc?
• second, when I try to XOR the lines I just mentioned, they get XORred only once, so I end up with a XORred line and a normal one for each duplicate line.

Answer 1

 p = (char *) malloc((strlen(ln) + 1) * sizeof(char));

instead of

 p = (char *) malloc(strlen(ln) * sizeof(char));

BTW, you can change

p = (char *) malloc((strlen(ln)+1) * sizeof(char));
strcpy(p, ln);
s[i++] = p;

by

s[i++] = strdup(ln)

It's the same

Answer 2

The malloc in filetostr isn't quite right. It should be

p = malloc(strlen(ln) + 1);

You need to allocate space for the terminating null character, don't need to cast the return and can rely on sizeof(char) being 1