How to make a copy of a string and return its address, assign that to a pointer and print the new string in C?

Question

I'm writing a function that gets a string, allocates memory on the heap that's enough to create a copy, creates a copy and returns the address of the beginning of the new copy. In main I would like to be able to print the new copy and afterwards use free() to free the memory. I think the actual function works although I am not the char pointer has to be static, or does it?

The code in main does not work fine...

#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <math.h>

int make_copy(char arr[]);

int main()
{
    char arrr[]={'a','b','c','d','e','f','\0'};

    char *ptr;
    ptr=make_copy(arrr);

    printf("%s",ptr);
    getchar();

    return 0;
}


int make_copy(char arr[])
{
    static char *str_ptr;
    str_ptr=(char*)malloc(sizeof(arr));

    int i=0;
    for(;i<sizeof str_ptr/sizeof(char);i++)
        str_ptr[i]=arr[i];

    return (int)str_ptr;
}

OK, so based on the comments. A revised version:

#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>

char* make_copy(char arr[]);

int main()
{
    char arrr[]={"abcdef\0"};

    char *ptr=make_copy(arrr);

    printf("%s",ptr);
    getchar();

    return 0;
}


char* make_copy(char arr[])
{
    static char *str_ptr;
    str_ptr=(char*)malloc(strlen(arr)+1);

    int i=0;
    for(;i<strlen(arr)+1;i++)
        str_ptr[i]=arr[i];

    return str_ptr;
}

Or even better:

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

char* make_copy(char arr[]);

int main()
{
    char arrr[]={"abcdef\0"};

    printf("%s",make_copy(arrr));
    getchar();

    return 0;
}


char* make_copy(char arr[])
{
    char *str_ptr;
    str_ptr=(char*)malloc(strlen(arr)+1);
    return strcpy(str_ptr,arr);
}

Why would you **ever** declare your function to return `int` when it needs to return a `char *`? Also, `sizeof(arr)` doesn't do what you think it does, `sizeof(char)` is always one, `sizeof(str_ptr)` doesn't do what you think it does either, you are missing a NUL terminator, and the `make_copy()` function leaks memory. That's all. You need to **learn C** before trying to write programs in it. — , Jan 27 '14 at 08:50
I was asked to return the address of the allocated space... I could have returned a char pointer which would made things easier, but that's not what I was requested. — user34920, Jan 27 '14 at 08:51
@user34920 The address of the allocated pointer is `&str_pointer`, but it's **still** not an `int`. It's a `char **`. What are you even trying to do with that poor `int`? I mean, how come? Just **use your common sense, man.** — , Jan 27 '14 at 08:52
`sizeof(char)` is always 1. This function exists in the standard library under the name `strdup`. Returning `char *` is exactly what you are asked to do. It is an address. — Marian, Jan 27 '14 at 08:53
This question appears to be off-topic because it lacks fundamental knowledge of the language being used. — , Jan 27 '14 at 08:54
@user34920 I did. For some reason, you didn't respect our advice and you are still casting the return value of malloc, which is bad. Also, `char arr[] = { "abcdef\0" }` should be `char arr[] = "abcdef";` -- tue trailing extra 0 is superfluous, so are the extraneous curly braces (a string literal is itself an array and can act as an initialzer list without braces). — , Jan 27 '14 at 09:53
@H2CO3 You are against casting the return value of malloc because it's a non prototyped function? Perhaps a very old compiler would assume the return type is int and that would result in casting an int to a pointer and if the pointer type is larger (wider?) than an int something bad might happen. However seems like GCC (which I am using) knows malloc, even without stdin. To be fair it does give a warning but works fine. — user34920, Jan 27 '14 at 10:38
@user34920 no, that's not the reason. the cast is harmful. warnings should always be fixed. Read the answer Unwind linked to in is answer. — , Jan 27 '14 at 10:46
@H2CO3 Just to be clear, a warning pops up when stdio.h is not included, that should not be left that way. agreed. Otherwise no warning. I didn't understand where you suggest the answer is. — user34920, Jan 27 '14 at 10:54
@H2CO3 - I think the reason I gave might actually be a better argument. Not including stdio.h and casting the return value of malloc would probably won't cause any issues. Size mismatch might actually cause a run-time crash. — user34920, Jan 27 '14 at 12:22

score 2 · Accepted Answer · edited May 23 '17 at 12:07

2

You're on the right track, but there are some issues with your code:

Don't use int when you mean char *. That's just wrong.
Don't list characters when defining a string, write char arrr[] = "abcdef";
Don't scale string alloations by sizeof (char); that's always 1 so it's pointless.
Don't re-implement strcpy() to copy a string.
Don't cast the return value of malloc() in C.
Don't make local variables static for no reason.
Don't use sizeof on an array passed to a function; it doesn't work. You must use strlen().
Don't omit including space for the string terminator, you must add 1 to the length of the string.

UPDATE Your third attempt is getting closer. :) Here's how I would write it:

char * make_copy(const char *s)
{
  if(s != NULL)
  {
    const size_t size = strlen(s) + 1;
    char *d = malloc(size);
    if(d != NULL)
      strcpy(d, s);
    return d;
  }
  return NULL;
}

This gracefully handles a NULL argument, and checks that the memory allocation succeeded before using the memory.

edited May 23 '17 at 12:07

Community

1
1

answered Jan 27 '14 at 08:55

unwind

391,730
64
469
606

This is shameless self-advertisement! Linking to your own answer! :P – Jan 27 '14 at 08:56
@H2CO3 I prefer to think of it as supporting arguments. :) Many people seem hardwired to do that cast, I find it helpful to actually provide arguments but don't want to repeat myself all over the place. – unwind Jan 27 '14 at 08:57
I wasn't being serious :P perhaps that answer of yours is my single favorite post on Stack Overflow (apart from Jon Skeet facts, of course). – Jan 27 '14 at 08:58
Gee, thanks! :) It was just downvoted, I guess that wasn't you then. :) I'll take a positive comparison to The Skeet any day, thankyouverymuch. – unwind Jan 27 '14 at 08:59
Oh gosh. I see, it was downvoted... some people just **really** don't know what they are doing (and, first and foremost, what they *should* be doing... too much "C/C++" "programmers" out there...) – Jan 27 '14 at 09:01

sujin · Answer 2 · 2014-01-27T08:58:55.880

0

sizeof(arr) will not give the exact size. pass the length of array to the function if you want to compute array size.

When pass the array to function it will decay to pointer, we cannot find the array size using pointer.

edited Jan 27 '14 at 08:58

answered Jan 27 '14 at 08:53

sujin

2,813
2
21
33

Thanks. Yes i mean array size using pointer. i will edit the answer. – sujin Jan 27 '14 at 08:55

score 0 · Answer 3 · answered Jan 27 '14 at 08:56

First, don't use sizeof to determine the size of your string in make_copy, use strlen.

Second, why are you converting a pointer (char*) to an integer? A char* is already a pointer (a memory address), as you can see if you do printf("address: %x\n", ptr);.

score 0 · Answer 4 · answered Jan 27 '14 at 09:21

0

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

char *strdup(const char *str)
{
    char *s = (char*)malloc(strlen(str)+1);
    if (s == NULL) return NULL;
    return strcpy(s, str);
}

int main()
{
    char *s = strdup("hello world");
    puts(s);
    free(s);
}

answered Jan 27 '14 at 09:21

cybermage14

168
1

that's pretty similar to the edited version I added, however you also covered a NULL case. BTW - writing char *strdup(const char *str) or just char *strdup(char str[]) is the same right? I understand that the pointer to an array is a const and perhaps the compiler will note something about it, however it does not matter here and *str is the same is str[] since C knows both are pointers right? – user34920 Jan 27 '14 at 09:32
the string "hello world" is a pointer to a character array literal, therefore it's type is const char *. Non-constant character arrays will also work with this function definition. Your compiler might throw a warning if you use char str[] and you pass in a literal string. – cybermage14 Jan 27 '14 at 09:36

score -1 · Answer 5 · answered Jan 27 '14 at 08:56

Points ~ return char* inside of int. ~ you can free the memory using below line

 if(make_copy!=NULL)
  free(make_copy)

Below is the modified code.

#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <math.h>
char* make_copy(char arr[]);

int main()
{
    char arrr[]={'a','b','c','d','e','f','\0'};

    char *ptr;
    ptr=make_copy(arrr,sizeof(arrr)/sizeof(char));

    printf("%s",ptr);
    printf("%p\n %p",ptr,arrr);
    getchar();

    return 0;
}


char* make_copy(char arr[],int size)
{
     char *str_ptr=NULL;
    str_ptr=(char*)malloc(size+1);

    int i=0;
    for(;i<size;i++)
        str_ptr[i]=arr[i];
       str_ptr[i]=0;

    return str_ptr;
}

How to make a copy of a string and return its address, assign that to a pointer and print the new string in C?

5 Answers5