Doug Molineux
Doug Molineux

Reputation: 12431

Unhandled exception for strlen in C

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:

Using strcat in C

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

Answers (5)

user3651790
user3651790

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

Paul R
Paul R

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

JeremyP
JeremyP

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

Konrad Rudolph
Konrad Rudolph

Reputation: 545588

  • You forgot one argument
  • sizeof(char) is 1 by definition
  • your realloc code is broken
  • strcat doesn’t take a char as its second argument
  • I’d just return the newly created string, like strcat does
char* 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:

  1. You must call mystrcat with a valid, initialised pointer – same as strcat!
  2. 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

wildplasser
wildplasser

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

Related Questions