Reputation: 12431
I have been stuck on this one for a while, I'm not an expert in C. Basically, I am trying to make a function that "safely" strcats a character to an existing char *.
I am trying to get the "dynamic allocation" method working from this example:
I have made a few modifications, I removed the var that's set by the realloc function (the compiler said that it returned void). I also modified it to only append one character instead of an array of characters. I figured this would change the "realloc" parameters, so instead of passing the length of the addition string, I just passed in "sizeof(char)" (x2 because the original had an extra sizeof char, i think because of the null terminator?)
char *buffer = NULL;
int mystrcat(char addition)
{
realloc(buffer, strlen(buffer) + sizeof(char)*2);
if (!buffer)
return 0;
strcat(buffer, addition);
return 1;
}
I call it like this:
if(!safestrcat(str[i+j]))
printf("Out of Memory");
For some reason, I am seeing this:
Unhandled exception at 0x60f0d540 (msvcr100d.dll) in myProg.exe: 0xC0000005: Access violation reading location 0x00000000.
And the debugger shows me strlen.asm at line 81:
main_loop:
mov eax,dword ptr [ecx] ; read 4 bytes
I'm sorry if this is a newb question, but what is happening? Why is the addition char not being appending to the buffer?
Sorry I should add, that it compiles succesfully.
Upvotes: 1
Views: 4050
Reputation:
char *mystrcat(char *buffer, char addition) {
char *bb;
size_t ll;
ll = buffer ? strlen(buffer) : 0;
bb = realloc(buffer, ll + 2);
if(!bb){
fprintf(stderr, "Memory exhausted in function: mystrcat !\n");
exit(EXIT_FAILURE);
}
buffer = bb; // Safe!!!
buffer[ll] = addition;
buffer[ll+1] = '\0';
return buffer;
}
Something like that. So if no memory most likely you can't do anything at all until finish your application. If only your system not be shutdown.
It's a critical error! But you can see this message. In logs surely. No guaranties to work correctly if you pass bad prt buffer. For example if your forgot to set null terminator. Error occurs at strlen!
Upvotes: 0
Reputation: 212969
Your call to realloc is completely broken - you need to check for success and then reassign the result of the function to your existing pointer.
You also need a char *
to pass as the second parameter to strcat
, not a char
.
Change:
int mystrcat(char addition)
{
realloc(buffer, strlen(buffer) + sizeof(char)*2);
if (!buffer)
return 0;
strcat(buffer, addition);
return 1;
}
to:
int mystrcat(char addition)
{
char st[2] = { addition, '\0' }; // make temporary string to hold `addition`
int len = buffer != NULL ? strlen(buffer) : 0; // NB: handle case where `buffer` has not yet been allocated
char * tmp = realloc(buffer, len + 2); // increase size of `buffer`
if (!tmp) // handle realloc failure
return 0;
buffer = tmp;
strcat(buffer, st); // append `addition`
return 1;
}
Upvotes: 2
Reputation: 86651
Loads of really good advice has been given in the other answers, but the reason that you are getting the access violation is because buffer
starts out as NULL
. Then you do strlen(buffer)
. strlen()
works by counting the characters starting from the address passed in until it gets to a '\0'
. So in your case, the first time in, you dereference a null pointer.
Upvotes: 2
Reputation: 545588
sizeof(char)
is 1 by definitionrealloc
code is brokenstrcat
doesn’t take a char
as its second argumentstrcat
doeschar* mystrcat(char* buffer, char addition) {
unsigned oldlen = strlen(buffer);
buffer = realloc(buffer, oldlen + 2);
if (buffer == NULL)
return NULL;
buffer[oldlen + 0] = addition;
buffer[oldlen + 1] = '\0';
return buffer;
}
However, pay attention to two things:
mystrcat
with a valid, initialised pointer – same as strcat
!In the case of failure, the function returns NULL
– in that case, it’s the caller’s responsibility to ensure that the original buffer’s memory is freed. This means that you mustn’t call the function as
buffer = mystrcat(buffer, 'x');
– This may cause a memory leak.
So a correct usage would be:
char* something = "hello";
char* buffer = malloc(sizeof(something) + 1);
strcpy(buffer, something);
char* new_buffer = mystrcat(buffer, 'x');
if (new_buffer == NULL) {
free(buffer);
exit(1);
}
buffer = new_buffer;
Yes, convoluted. This is the price for safe memory operations.
Upvotes: 6
Reputation: 44250
char * mystrcat(char *str, char addition)
{
size_t len;
len = strlen(str);
str = realloc(str, len + 2);
if (!str)
return NULL; /* ... */
str[len++] = addition;
str[len] = 0;
return str;
}
Upvotes: 1