Search code examples
cmemory-managementcstringnull-terminatedstrdup

How could this C fragment be written more safely?


I have the following C code fragment and have to identify the error and suggest a way of writing it more safely:

char somestring[] = "Send money!\n";
char *copy;

copy = (char *) malloc(strlen(somestring));

strcpy(copy, somestring);
printf(copy);

So the error is that strlen ignores the trailing '\0' of a string and therefore it is not going to be allocated enough memory for the copy but I'm not sure what they're getting at about writing it more safely?

I could just use malloc(strlen(somestring)+1)) I assume but I'm thinking there must be a better way than that?


EDIT: OK, I've accepted an answer, I suspect that the strdup solution would not be expected from us as it's not part of ANSI C. It seems to be quite a subjective question so I'm not sure if what I've accepted is actually the best. Thanks anyway for all the answers.


Solution

  • char   somestring[] = "Send money!\n";
    char   *copy;
    size_t copysize;
    
    copysize = strlen(somestring)+1;
    copy = (char *) malloc(copysize);
    if (copy == NULL)
        bail("Oh noes!\n");
    
    strncpy(copy, somestring, copysize);
    printf("%s", copy);
    

    Noted differences above:

    • Result of malloc() must be checked!
    • Compute and store the memory size!
    • Use strncpy() because strcpy() is naughty. In this contrived example it won't hurt, but don't get into the habit of using it.

    EDIT:

    To those thinking I should be using strdup()... that only works if you take the very narrowest view of the question. That's not only silly, it's overlooking an even better answer:

    char somestring[] = "Send money!\n";
    char *copy = somestring;
    printf(copy);
    

    If you're going to be obtuse, at least be good at it.