Reputation: 172
When I try to execute the following code in Visual Studio I get a data access error. I looked it up but saw that it happens when I try to modify a string literal which is a read only.
char *my_strcpy(char *dest, const char *source)
{
int i;
while(*dest)
{
*dest++ = *source++;
}
return dest;
}
int main()
{
char str[80] = "Hello, there!";
char *strPtr = NULL;
my_strcpy (strPtr, str);
return 0;
}
error:
Unhandled exception at 0x01163C91 in PtrStrArr.exe: 0xC0000005: Access violation reading location 0x00000000.
I am unable to identify where I am modifying a string literal. I even declared the array as a constant. If I remove the declaration
char *strPtr = NULL;
I get an error
Run-Time Check Failure #3 - The variable 'strPtr' is being used without being initialized.
Could you please point me to my mistake here?
EDIT:
Thanks to all the answers I was able to correct my code and get the ouput as below.
#include <stdio.h>
#include<malloc.h>
char *my_strcpy(char *dest, const char *source)
{
char *retVal = dest;
int i=0;
while(*source != '\0')
{
*dest++ = *source++;
}
*dest++ = '\0';
return retVal;
}
void print_array(char *str)
{
int i;
while(*str)
{
printf("%c",*str++);
}
}
int main()
{
char str[80] = "What up dude?";
char* strPtr;
strPtr = (char *)(malloc(sizeof str));
print_array (str);
printf("\n");
my_strcpy (strPtr, str);
print_array (strPtr);
return 0;
}
I do have one question though. In all your answers you have not made the cast from void* to char* when using malloc. But when I tried it out I had to explicitly
strPtr = (char *)(malloc(sizeof str));
I am using malloc in the right way? Thanks to every one for your answers.
Upvotes: 0
Views: 6371
Reputation: 16424
it happens when I try to modify a string literal which is a read only
That's just one of the many ways it can happen. Another is trying to store into NULL
, which is what you're doing here:
char str[80] = "Hello, there!";
char *strPtr = NULL;
my_strcpy (strPtr, str);
You want to copy str
to somewhere, but where? You haven't provided any memory to store into.
Consider
char str[80] = "Hello, there!";
char str2[sizeof str];
my_strcpy (str2, str);
or
char str[80] = "Hello, there!";
char* strptr = malloc(sizeof str);
if (!strptr)
/* do something on out-of-memory */;
my_strcpy (strptr, str);
Also, my_strcpy
is looping until it finds the NUL
in the destination, which is of course wrong ... you can't look at the destination until you've stored something there. And, you aren't storing the NUL into the destination. Consider the simple loop
while ((*dest++ = *source++) != '\0')
;
Finally, you probably don't want to return dest
, which now points past the NUL
. Either declare my_strcpy
to return
void
and don't return anything, or if you want it to have the same semantics as strcpy
, save the original value of dest
and return
that, e.g.,
char* retval = dest;
while ((*dest++ = *source++) != '\0')
;
return retval;
or
for (char* destp = dest; (*destp++ = *source++) != '\0';)
;
return dest;
Pick the style you prefer.
Upvotes: 4
Reputation: 58281
First:
You don't allocate memory for dest
array:
char *strPtr = NULL; <-- allocate memory here
// strPtr = malloc(lenght + 1); // +1 for \0 chaar
my_strcpy (strPtr, str);
Second:
Also, in string copy function loop should run while *source
is nul.
while(*source){
Third:
You don't add \0
at the end to dest
after loop-ends, add *dest = '\0';
after while loop in string copy code.
Fourth:
You returns dest
while in loop you increments it!
Try this code:
char * my_strcpy(char *dest, const char *source)
{
dest = malloc(strlen(source) + 1); //allocate memory
tdest = dest; // store it to return
while(*source) // copy until source terminate at \0
{
*tdest++ = *source++;
}
*tdest = '\0'; // add nul termination in dest string
return dest; // return allocated memory address
}
Note that memory allocation is done inside string copy function.
By the way, I didn't change your function prototype and function name but what are you trying to build is called strdup()
. Because memory allocation is done inside function and duplicate string is returned so you don't need to pass first argument.
So as @Jim Balter also reviewed my answer semantically correct way to write function is:
char * my_strdup(const char *source)
{
dest = malloc(strlen(source) + 1); //allocate memory
tdest = dest; // store it to return
while(*source) // copy until source terminate at \0
{
*tdest++ = *source++;
}
*tdest = '\0'; // add nul termination in dest string
return dest; // return allocated memory address
}
call it as:
char* strPtr = my_strdup(str);
It you want memory allocation in main function and keep string copy function simple then please read Jim Balter's answer.
Upvotes: 5
Reputation: 4758
You will have to allocate memory for strPtr
before calling your copy function
strPtr=malloc(sizeof(str));
Upvotes: 2