Copying strings and pointers issue in C

Question

In this exrecie I get an input and output files. the input file contains a number and then a strings of length no longer than 10, I need to sort them and output them in the output file. I defined a char** arrStr which contains all strings of size at most 10.

I'm trying to understand if the following code would work (for some reason I can't run it on Eclipse) my main concern is about copying the strings correctly and not losing the information. I put a note "is this ok?" next to the statement which concerns me the most, and I'd appreciate any other corrections.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>

int comp(const void* p1, const void* p2) {
    return strcmp((char*)p1, (char*) p2);
} 

int main(int argc, char** argv) {
    FILE* fin;
    FILE* fout;
    int N;
    char** arrStr;
    char str[11];
    int i;

    if (argc!=3) {
        printf("Please enter the program's name and two paths");
        assert(0);
    }

    fin=fopen(argv[1], "r");
    if (fin==NULL) {
        printf("path 1 is not valid");
        assert(0);
    }

    fout=fopen(argv[2], "w");
    if (fout==NULL) {
        printf("path for file 2 is not valid");
        fclose(fin);
        assert(0);
    }

    fscanf(fin, "%d", &N); 
    arrStr=(char**)calloc(N, sizeof(char)*11);
    for (i=0; i<N; i++) { 
        fscanf(fin, "%s", str);
        strcpy(arrStr[i], str);  /*is it ok?*/
    }

    qsort(arrStr, N, sizeof(char)*11, comp);
    for (i=0; i<N; i++) {
        if (i==N-1)
            fprintf(fout, "%s", arrStr[i]);
        else
            fprintf(fout, "%s,", arrStr[i]);

    }
    fclose(fin);
    free(arrStr);
    fclose(fout);
    return 1;
}

score 0 · Answer 1 · answered Sep 18 '12 at 10:34

0

The line

strcpy(arrStr[i], str);  /*is it ok?*/

is not okay. You copy into a NULL pointer.

For this case use e.g. strdup instead of strcpy:

arrStr[i] = strdup(str);

Don't forget to free this string later.

You are also allocating the array wrong:

arrStr=(char**)calloc(N, sizeof(char)*11);

This doesn't allocate an array of strings, it allocates N * 11 bytes. Change to:

arrStr = calloc(N, sizeof(char *));

answered Sep 18 '12 at 10:34

Some programmer dude

400,186
35
402
621

Thank you for the answer, can I initialize arrStr[i] with " " instead of using strdup? another thing, with you allocating-where does the fact that each string is no more than 11 chars is mentioned? is it not important? – Joni Sep 18 '12 at 10:46
@Joni No, then you will try to copy over a string literal which is read only, and not of the correct length. Also, do the string have 10 or 11 characters? If 11, then you need to make `str` 12 characters, one extra for the special string terminator character. – Some programmer dude Sep 18 '12 at 10:50
@Joni And if the string you read is 11 characters, then `strdup` will allocate enough for that string but not more (plus one byte for the terminator or course). – Some programmer dude Sep 18 '12 at 10:51
it is 10 length, and that is why I put 11. I still don't understand why allocation of 11*N bytes is wrong, you know that you have N words and ecah one is no longer then 10 bytes long. – Joni Sep 18 '12 at 10:55
@Joni A proper array (or array of arrays) is a contiguous area of memory, a dynamic "array" of string is _not_, it's an "array" of _pointers_. So the `calloc` call allocates memory for the _pointers_ inside the array, then you have to allocate the actual pointers and make them point to the strings. – Some programmer dude Sep 18 '12 at 10:59
Thank you. Do you think of another way of doing this easily? – Joni Sep 18 '12 at 11:08
I would not recommend strdup as it isn't a standard function. – Lundin Sep 18 '12 at 11:08
@Lundin It's not a standard C function, only POSIX. However it do exist on all platforms I ever worked on (POSIX or not). – Some programmer dude Sep 18 '12 at 11:14
@JoachimPileborg There are plenty of embedded system platforms where it doesn't exist. More importantly, it is completely redundant as standard functions can perform the same task. We really can't make any assumptions about that OP's platform, except that it is running on some sort of OS. – Lundin Sep 18 '12 at 11:22
what is the way for doing it straight forward without strdup? maybe to allocate place for each (arrStr[i]) dynamiclly and then to use strcpy? – Joni Sep 18 '12 at 11:23
@Joni strlen to get the length, then malloc/calloc to allocate string length + 1 characters, then memcpy/strcpy. – Lundin Sep 18 '12 at 11:23
So basically I allocate memory to each arrStr[i]=(char*)calloc(strlen(str)+1,sizeof(char)); separately and use strcpy(arrStr[i],str); for every i. By free arrStr do I free all of the strings or should I run one by one and free it? Thanks a lot! – Joni Sep 18 '12 at 11:31
@Joni Yes that it, but if there is a `strdup` function you don't have to do it, it's named `_strdup` in Windows. See also [this question](http://stackoverflow.com/questions/7582394/strdup-or-strdup). As for freeing, you have to free all separate string one by one in a loop. – Some programmer dude Sep 18 '12 at 11:39

Copying strings and pointers issue in C

1 Answers1