0

I have a problem with below function.

When I try to realloc() memory I get more than I actually asked for!

In this instance I try to concatenate 2 strings, one who is 14 characters long and one who is 11 characters long, but the end result is that memTemp is 38 characters long even though memNewSize shows that it is in fact 25, does anyone know what to do?

int dstring_concatenate(DString* destination, DString source)
{
    assert(destination != NULL); // Precondition: destination ar ej NULL
    assert(*destination != NULL); // Precondition: *destination ar ej NULL
    assert(source != NULL); // Precondition: source ar ej NULL
    //Dstring looks like this = "typedef char* Dstring;"

    int memNewSize = strlen(*destination) + strlen(source);
    char *memTemp;
    memTemp = (char*)realloc(memTemp, sizeof(char) * memNewSize);

    printf("%d\n", memNewSize);
    printf("%d\n", strlen(memTemp));

    if(memTemp == NULL)
    {
        printf("Could not allocate new memory.\n");
        return 0;
    }
    else
    {
        *destination = memTemp;
        strcat(*destination, source);
        return 1;
    }
}
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
Erik.E
  • 39
  • 2
  • 2
  • 6
  • Of course the title should've been "realloc allocates too much memory" – Erik.E Nov 08 '17 at 07:18
  • Then why not edit it? – M Oehm Nov 08 '17 at 07:18
  • what is `DString`? – Sourav Ghosh Nov 08 '17 at 07:19
  • 2
    Should it not be `memNewSize + 1` to include the string terminator? – Some programmer dude Nov 08 '17 at 07:21
  • As for your problem, would it not make more sense to pass `*destination` to the `realloc` call? – Some programmer dude Nov 08 '17 at 07:22
  • Are you hiding pointer with typedef `DString` ? it's **very very** bad idea. By the way, your question is lack of [mcve], I vote to close. – Stargateur Nov 08 '17 at 07:31
  • Shouldn't the argument to `realloc` be the old pointer? – qwn Nov 08 '17 at 07:41
  • @Erik.E - I think the suggestion regarding the title pertains to somewhat of a title misconception. E.g "`realloc` is doing something wrong." Or, "some `C-library function` is doing something wrong..." 99.999% of the time, it is not the library function that is at fault. – David C. Rankin Nov 08 '17 at 07:45
  • How do you know that "`memTemp` is 38 characters long"? Seems to me that you are attempting an `strlen` on uninitialised memory. `strlen` will just run along looking for a binary zero`. Eventually it will either find one or run into an illegal address, either way the reported length will be wrong. – cdarke Nov 08 '17 at 07:49
  • No, realloc doesn't allocate "more memory than requested". Ignoring all the other bugs that have been pointed out, there are no standard methods for you to find out how much memory realloc gave you. Calling strlen on uninitialized memory is definitely not the way to do it. – Art Nov 08 '17 at 07:54

2 Answers2

6

The problem here is, realloc() works (properly) on only

  • pointers earlier returned by allocator function
  • NULL pointers.

Quoting C11, chapter §7.22.3.5

If ptr is a null pointer, the realloc function behaves like the malloc function for the specified size. Otherwise, if ptr does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to the free or realloc function, the behavior is undefined. [....]

In your case, memTemp (being an automatic storage local scoped variable) is just an unitialized pointer, with an indeterminate value, pointing to who knows what address! It's not even guaranteed to be NULL. So, all you have is undefined behavior.

Just a guess: Probably you meant to initialize memTemp with incoming *destination?


That said, as pointed out in comments under the actual question,

  • The size multiplier supplied in realloc() should be memNewSize + 1 to be able to hold the null-terminator.
  • sizeof(char) is guaranteed to be 1 in C, so using it as a multiplier is redundant.
Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
  • Do you have a user script to one-click-quote a section from the standard :p – Ajay Brahmakshatriya Nov 08 '17 at 07:25
  • 1
    @AjayBrahmakshatriya :) Not really, it's just a bad habit I developed myself to avoid misunderstanding by using wrong wordings (English is not my native language and believe me, I had been in situations where what I wrote was 180 degrees what what I meant!!) – Sourav Ghosh Nov 08 '17 at 07:29
  • Hey there Sourav, thanks for the fast answer! Dstring is a typedef that is a char pointer, so Dstring *destination is actually a char double pointer, and *destination points already to an dynamically allocated string, so should memTemp be a problem if I use *destination in my realloc() call? – Erik.E Nov 08 '17 at 07:34
  • 1
    @Erik.E it is generally a good idea not to hide pointers in typedefs – Ajay Brahmakshatriya Nov 08 '17 at 07:36
  • @Erik.E No, there should not be any problem if I understood your version correctly. – Sourav Ghosh Nov 08 '17 at 07:36
  • ..and I second what @AjayBrahmakshatriya said, it's really a _bad_ thing. – Sourav Ghosh Nov 08 '17 at 07:37
  • @AjayBrahmakshatriya Sorry, I got this code from my teacher, my job was to implement the different functions that comes with it :/ – Erik.E Nov 08 '17 at 07:39
  • @SouravGhosh So what could be the problem then? If newMemSize is 25, and *destination points to an already allocated string, Why does memTemp end up being 38 characters big? – Erik.E Nov 08 '17 at 07:42
  • @Erik.E it should not be. Point: `*destination points to an already allocated string`..it must be returned via allocator functions, otherwise, we're back to square one. – Sourav Ghosh Nov 08 '17 at 07:45
0

Alright, I got it to work, thanks so much guys! Updated code below.

int dstring_concatenate(DString* destination, DString source)
{
    assert(destination != NULL); // Precondition: destination ar ej NULL
    assert(*destination != NULL); // Precondition: *destination ar ej NULL
    assert(source != NULL); // Precondition: source ar ej NULL
    // Dstring look like this = "typedef char* Dstring;"

    int memNewSize = strlen(*destination) + strlen(source);
    char *memTemp;
    memTemp = (char*)realloc(*destination, sizeof(char) * memNewSize+1);

    if(memTemp == NULL)
    {
        printf("Could not allocate new memory.\n");
        return 0;
    }
    else
    {
        *destination = memTemp;
        strcat(*destination, source);
        return 1;
    }
}
Erik.E
  • 39
  • 2
  • 2
  • 6
  • Maybe `memTemp = realloc (*destination, sizeof *memTemp * memNewSize+1);`? See: [**Do I cast the result of malloc?**](http://stackoverflow.com/q/605845/995714) – David C. Rankin Nov 08 '17 at 07:52