multiple realloc w/ malloc blows away data

Discussion in 'C Programming' started by WL, Jul 14, 2003.

  1. WL

    WL Guest

    Hey, all. I'm creating an array of strings (char **argv style) on
    the fly, and using realloc to create string pointers, and malloc
    for the strings itself (if that makes any sense).

    I'm using the construct
    ptr = realloc(ptr, size);
    *ptr = malloc(string_length);
    strncpy(ptr, src, string_length);
    to call realloc() multiple times. This should be ok, right?

    Anyways, the problem I'm seeing is in (no error checking)

    char **template, **temp_ptr;

    /* initial template malloc */

    template = realloc(template, 2 * sizeof **template);
    temp_ptr = template + 1;
    *temp_ptr = malloc(5);
    strncpy(*temp_ptr, "abcd", 5);
    printf("%s\n%s\n%d %d\n", *template, *(template+1), template, temp_ptr);

    template = realloc(template, 3 * sizeof **template);
    temp_ptr = template + 2;
    *temp_ptr = malloc(5);
    strncpy(*temp_ptr, "wxyz", 5);
    printf("%s\n%s\n%s\n%d %d\n", *template, *(template+1), *(template+2), template, temp_ptr);

    and the associated output is

    1234
    abcd
    536952928 536952936
    @
    abcd
    wxyz
    536952928 536952944

    After some futzing, it seems the data in *template gets blown away
    after the last malloc(). I've looked this over all morning, but I
    don't see where I'm going wrong.

    The compiler is gcc 2.95.3 on alpha/netbsd 1.6.1.

    WL
    --
    real mail: wliao at sdf loSnPesAtarM org
    (remove the uppercase letters...)
     
    WL, Jul 14, 2003
    #1
    1. Advertising

  2. In article <bev6cr$957j7$-berlin.de>, WL wrote:
    > Hey, all. I'm creating an array of strings (char **argv style) on
    > the fly, and using realloc to create string pointers, and malloc
    > for the strings itself (if that makes any sense).
    >
    > I'm using the construct
    > ptr = realloc(ptr, size);
    > *ptr = malloc(string_length);
    > strncpy(ptr, src, string_length);
    > to call realloc() multiple times. This should be ok, right?
    >
    > Anyways, the problem I'm seeing is in (no error checking)
    >
    > char **template, **temp_ptr;
    >
    > /* initial template malloc */
    >
    > template = realloc(template, 2 * sizeof **template);

    [-]
    Which means template = realloc( template, 2 * sizeof (char) ) and
    since you didn't set template to NULL realloc() doesn't do the initial
    allocation for you but presumes template, which may be 0xdeadbeef or
    anything else here, to be a valid address.

    So it'd read ...
    char** template = NULL;

    template = realloc( template, 2 * sizeof (*template) );

    .... to start with.
    [-]
    Ta',
    Juergen

    --
    \ Real name : Juergen Heinzl \ no flames /
    \ EMail Private : \ send money instead /
     
    Juergen Heinzl, Jul 14, 2003
    #2
    1. Advertising

  3. WL wrote:
    > Hey, all. I'm creating an array of strings (char **argv style) on
    > the fly, and using realloc to create string pointers, and malloc
    > for the strings itself (if that makes any sense).
    >
    > I'm using the construct
    > ptr = realloc(ptr, size);
    > *ptr = malloc(string_length);
    > strncpy(ptr, src, string_length);
    > to call realloc() multiple times. This should be ok, right?
    >
    > Anyways, the problem I'm seeing is in (no error checking)
    >
    > char **template, **temp_ptr;
    >
    > /* initial template malloc */
    >
    > template = realloc(template, 2 * sizeof **template);

    Ouch ouch ouch.
    There are two problems with this line.
    1) The alloc'ed size is plain wrong. Think. template is declared as a
    char **, right ? Thus **template has type char. Hence sizeof **template
    is exactly 1. I don't think that's what you want.

    2) That said, remember that realloc, just like malloc, can fail. If it
    fails, it returns a NULL pointer. Thus, with your statement above you
    are actually erasing the only reference you had to some alloc'ed data.
    You thus can't free it anymore. I don't think that's what you want either.

    The correct way to do what you want is:

    temp_ptr = realloc(template, 2 * sizeof *template);
    if (NULL == temp_ptr)
    {
    /* do some cleanup, possibly:
    free(template);
    although that's not likely */
    }
    else
    {
    template = temp_ptr;
    /* carry on with processing */
    }
    > temp_ptr = template + 1;
    > *temp_ptr = malloc(5);
    > strncpy(*temp_ptr, "abcd", 5);
    > printf("%s\n%s\n%d %d\n", *template, *(template+1), template, temp_ptr);
    >
    > template = realloc(template, 3 * sizeof **template);

    Keep in mind *alloc functions are likely to be expensive. Use them
    sparingly. Instead of increasing the size of your array of string one
    string pointer at a time, start with a fair size (to be determined
    depending on your application's requirements), keep track of the size,
    and when you run out of room, *double* the size !
    > temp_ptr = template + 2;
    > *temp_ptr = malloc(5);
    > strncpy(*temp_ptr, "wxyz", 5);
    > printf("%s\n%s\n%s\n%d %d\n", *template, *(template+1), *(template+2), template, temp_ptr);
    >


    --
    Bertrand Mollinier Toublet
    "Reality exists" - Richard Heathfield, 1 July 2003
     
    Bertrand Mollinier Toublet, Jul 14, 2003
    #3
  4. WL

    Eric Sosman Guest

    WL wrote:
    >
    > Hey, all. I'm creating an array of strings (char **argv style) on
    > the fly, and using realloc to create string pointers, and malloc
    > for the strings itself (if that makes any sense).
    >
    > I'm using the construct
    > ptr = realloc(ptr, size);
    > *ptr = malloc(string_length);
    > strncpy(ptr, src, string_length);
    > to call realloc() multiple times. This should be ok, right?


    No, not at all right. In the first line there's a problem
    if realloc() fails and returns NULL. Guess what? You've stored
    the NULL value into `ptr', and you can no longer find the memory
    `ptr' used to point to. This is called a "potential memory leak."

    Then in the second line, you always store the new pointer
    in the very first slot of the reallocated array, clobbering
    whatever lived there before. That is, the second time you do
    this you lose track of the string you stored the first time,
    and the third time you lose the string stored the second time,
    and so on. This is called a "big-time memory leak."

    The second and/or third lines may also be in error in the
    use of `string_length', whose initialization you haven't shown.
    If `string_length' is the strlen() of some string, then the
    copied string winds up lacking a terminating '\0' character --
    that is, it's no longer a "string" as defined by C, and you
    dare not attempt to use it as such.

    Finally, there's no error-checking. "Omitted for brevity,"
    I'm sure, but considering the rate at which errors are being
    committed in this code, brevity here seems to be the soul of
    something other than wit.

    > Anyways, the problem I'm seeing is in (no error checking)
    >
    > char **template, **temp_ptr;
    >
    > /* initial template malloc */
    >
    > template = realloc(template, 2 * sizeof **template);


    First, `template' has never been initialized to point to
    anything in particular. If it's at file scope it's implicitly
    initialized to NULL, and that's all right. But if it's local
    to a function, it "points to garbage" and you are in for big
    trouble.

    Second, see above for a reason to avoid `p = realloc(p,...)'.

    Third, that size calculation is wrong. R-O-N-G, wrong.
    Hint: the value is two. Not "two pointers' worth," but two.
    You want something like `2 * sizeof *template' instead.

    Fourth, "anyways" is not an English word.

    > temp_ptr = template + 1;
    > *temp_ptr = malloc(5);


    Since you only allocated two bytes' worth of memory above
    (if you even got that far, given that `template' was uninitialized
    when you called realloc() on it), it's quite likely that `temp_ptr'
    now points outside the allocated area. When you try to store the
    result of malloc() there, who knows what you might be stepping
    upon?

    > strncpy(*temp_ptr, "abcd", 5);


    Not an error, exactly, but this habit you've fallen into of
    using strncpy() instead of strcpy() is not a good one. The habit
    of using magic numbers like `5' and `2' is perhaps even worse.

    > printf("%s\n%s\n%d %d\n", *template, *(template+1), template, temp_ptr);


    You seem unconcerned about the lake of brimstone that awaits
    persistent sinners, because here comes another one. Another
    three, actually.

    First, what's stored in `*template'? Well, you never stored
    anything there, did you? So what reason have you to believe it
    points to the start of a valid string that you can use with the
    "%s" specifier? None at all, that's right ...

    Second and third, what makes you think you can print a pointer
    value with the "%d" specifier? "%d" is for integers, not for
    pointers. If it happens to do something halfway sensible on your
    particular system, that's pure dumb luck.

    > template = realloc(template, 3 * sizeof **template);


    Same errors in the previous realloc(). You're up to a whole
    three bytes now -- and note that if realloc() decided to copy the
    old stuff to a new location, it has not obligingly copied the
    things you stored outside the bounds of the old area ...

    > temp_ptr = template + 2;
    > *temp_ptr = malloc(5);
    > strncpy(*temp_ptr, "wxyz", 5);
    > printf("%s\n%s\n%s\n%d %d\n", *template, *(template+1), *(template+2), template, temp_ptr);


    All the same errors, all over again.

    > and the associated output is


    Considering all the problems, it's a bit surprising that
    you got any output at all.

    > 1234
    > abcd
    > 536952928 536952936
    > @
    > abcd
    > wxyz
    > 536952928 536952944
    >
    > After some futzing, it seems the data in *template gets blown away
    > after the last malloc(). I've looked this over all morning, but I
    > don't see where I'm going wrong.


    If you can't see what's wrong with what you've written, you
    are in serious need of a C course. Pointers are obviously a
    mystery to you, and memory management seems to be a dark art.
    Read Sections 4 through 7 -- ALL of it! -- in the comp.lang.c
    Frequently Asked Questions (FAQ) list

    http://www.eskimo.com/~scs/C-faq/faq.html

    .... and it would do you no lasting harm to read Section 8 as
    well. Until you improve your understanding, you are just
    wasting your time writing stuff that looks like code but lacks
    sensible content.

    --
     
    Eric Sosman, Jul 14, 2003
    #4
  5. WL

    Al Bowers Guest

    WL wrote:
    > Hey, all. I'm creating an array of strings (char **argv style) on
    > the fly, and using realloc to create string pointers, and malloc
    > for the strings itself (if that makes any sense).
    >
    > I'm using the construct
    > ptr = realloc(ptr, size);
    > *ptr = malloc(string_length);
    > strncpy(ptr, src, string_length);
    > to call realloc() multiple times. This should be ok, right?
    >
    > Anyways, the problem I'm seeing is in (no error checking)
    >
    > char **template, **temp_ptr;
    >
    > /* initial template malloc */
    >
    > template = realloc(template, 2 * sizeof **template);
    > temp_ptr = template + 1;
    > *temp_ptr = malloc(5);
    > strncpy(*temp_ptr, "abcd", 5);
    > printf("%s\n%s\n%d %d\n", *template, *(template+1), template, temp_ptr);
    >
    > template = realloc(template, 3 * sizeof **template);
    > temp_ptr = template + 2;
    > *temp_ptr = malloc(5);
    > strncpy(*temp_ptr, "wxyz", 5);
    > printf("%s\n%s\n%s\n%d %d\n", *template, *(template+1), *(template+2), template, temp_ptr);
    >
    > and the associated output is
    >
    > 1234
    > abcd
    > 536952928 536952936
    > @
    > abcd
    > wxyz
    > 536952928 536952944
    >
    > After some futzing, it seems the data in *template gets blown away
    > after the last malloc(). I've looked this over all morning, but I
    > don't see where I'm going wrong.
    >
    > The compiler is gcc 2.95.3 on alpha/netbsd 1.6.1.
    >


    Nothing is wrong with the compiler. You have the allocations
    and printfs in the code all messed up with undefined behavior.
    Here is an example of how you may want to proceed.

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

    int main(void)
    {
    char **template, **temp_ptr;
    unsigned i, count = 0;

    template = malloc((count+1)*sizeof(*template));
    if(template == NULL) /* Exit the program */
    exit(EXIT_FAILURE);
    if((template[count] = malloc(5)) == NULL)
    { /* Deallocate and exit */
    free(template);
    exit(EXIT_FAILURE);
    }
    strcpy(template[count],"abcd");
    count++;
    temp_ptr = realloc(template,(count+1)*sizeof(*template));
    if(temp_ptr == NULL)
    { /* Free the allocations and Exit */
    for(i = 0; i < count;i++) free(template);
    free(template);
    exit(EXIT_FAILURE);
    }
    template = temp_ptr;
    if((template[count] = malloc(5)) == NULL)
    {
    for(i = 0; i < count;i++) free(template);
    free(template);
    exit(EXIT_FAILURE);
    }
    strcpy(template[count],"wxyz");
    count++;
    for(i = 0;i < count;i++)
    printf("template[%u] = \"%s\"\n",i,template);

    /* Free the allocations */
    for(i = 0; i < count;i++) free(template);
    free(template);
    return 0;
    }



    --
    Al Bowers
    Tampa, Fl USA
    mailto: (remove the x)
    http://www.geocities.com/abowers822/
     
    Al Bowers, Jul 14, 2003
    #5
  6. WL

    Default User Guest

    Eric Sosman wrote:

    > Until you improve your understanding, you are just
    > wasting your time writing stuff that looks like code but lacks
    > sensible content.



    Oooo, dang that's good. If I had a .sig I'd snag that, but I don't so I
    won't.




    Brian Rodenborn
     
    Default User, Jul 15, 2003
    #6
  7. WL

    Wenchi Guest

    poor use of realloc blows away data (was: multiple realloc w/ malloc blows away data)

    Eric Sosman <> wrote:
    >WL wrote:
    >>
    >> Hey, all. I'm creating an array of strings (char **argv style) on
    >> the fly, and using realloc to create string pointers, and malloc
    >> for the strings itself (if that makes any sense).


    >
    >> strncpy(*temp_ptr, "abcd", 5);

    >
    > Not an error, exactly, but this habit you've fallen into of
    >using strncpy() instead of strcpy() is not a good one. The habit
    >of using magic numbers like `5' and `2' is perhaps even worse.


    Why is using strncpy instead of strcpy bad? Doesn't declaring the
    length keep the destination buffer safer compared to strcpy?

    Thanks for all the comments (in the other followups as well). After
    more reading and trying, here is what I ended up with, and it works
    on 2 specific compiler/platform combination I'm testing it on.

    Critique away :)

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

    int main(int ac, char **av)
    {
    char **template = NULL, **temp_ptr = NULL, **ptr = NULL;

    template = malloc(sizeof *template);
    if (template == NULL) { exit(1); }
    *template = malloc(5);

    if (*template == NULL) { exit(1); }
    strncpy(*template, "1234", 5);

    ptr = realloc(template, 2 * sizeof *template);
    if (ptr == NULL) { exit(1); }
    template = ptr;
    temp_ptr = ptr + 1;
    *temp_ptr = malloc(5);
    if (*temp_ptr == NULL) { exit(1); }
    strncpy(*temp_ptr, "abcd", 5);

    ptr = realloc(template, 3 * sizeof *template);
    if (ptr == NULL) { exit(1); }
    template = ptr;
    temp_ptr = ptr + 2;
    *temp_ptr = malloc(5);
    if (*temp_ptr == NULL) { exit(1); }
    strncpy(*temp_ptr, "wxyz", 5);

    printf("%s\n%s\n%s\n", *template, *(template+1), *(template+2));

    return 0;
    }

    WL
    --
    real mail: wliao at sdf loSnPesAtarM org
    (remove the uppercase letters...)
     
    Wenchi, Jul 15, 2003
    #7
  8. Re: poor use of realloc blows away data (was: multiple realloc w/ malloc blows away data)

    In article <beveb4$9g9sc$-berlin.de>,
    Wenchi <> wrote:

    > Eric Sosman <> wrote:
    > >WL wrote:
    > >>
    > >> Hey, all. I'm creating an array of strings (char **argv style) on
    > >> the fly, and using realloc to create string pointers, and malloc
    > >> for the strings itself (if that makes any sense).

    >
    > >
    > >> strncpy(*temp_ptr, "abcd", 5);

    > >
    > > Not an error, exactly, but this habit you've fallen into of
    > >using strncpy() instead of strcpy() is not a good one. The habit
    > >of using magic numbers like `5' and `2' is perhaps even worse.

    >
    > Why is using strncpy instead of strcpy bad? Doesn't declaring the
    > length keep the destination buffer safer compared to strcpy?


    Read _exactly_ what strncpy does. Take note of two things: What happens
    if the destination buffer is 10 Megabytes and you copy a five character
    string? (Ouch, that hurts. ) What happens if the destination buffer is
    five bytes and you copy a five character string? (Time bomb ticking. )
     
    Christian Bau, Jul 15, 2003
    #8
  9. WL

    Blah Guest

    Re: poor use of realloc blows away data (was: multiple realloc w/ malloc blows away data)

    "Wenchi" <> wrote
    > Thanks for all the comments (in the other followups as well). After
    > more reading and trying, here is what I ended up with, and it works
    > on 2 specific compiler/platform combination I'm testing it on.
    >
    > Critique away :)
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > int main(int ac, char **av)


    Style comment:
    main is typically 'int main(int argc, char **argv)'
    while ac and av are certainly not misleading, argc and argv are clearer.

    <remainder snipped>
     
    Blah, Jul 15, 2003
    #9
  10. WL

    Al Bowers Guest

    Re: poor use of realloc blows away data

    Wenchi wrote:


    > Thanks for all the comments (in the other followups as well). After
    > more reading and trying, here is what I ended up with, and it works
    > on 2 specific compiler/platform combination I'm testing it on.
    >
    > Critique away :)
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > int main(int ac, char **av)
    > {
    > char **template = NULL, **temp_ptr = NULL, **ptr = NULL;
    >
    > template = malloc(sizeof *template);
    > if (template == NULL) { exit(1); }
    > *template = malloc(5);
    >
    > if (*template == NULL) { exit(1); }
    > strncpy(*template, "1234", 5);
    >
    > ptr = realloc(template, 2 * sizeof *template);
    > if (ptr == NULL) { exit(1); }
    > template = ptr;
    > temp_ptr = ptr + 1;
    > *temp_ptr = malloc(5);
    > if (*temp_ptr == NULL) { exit(1); }
    > strncpy(*temp_ptr, "abcd", 5);
    >
    > ptr = realloc(template, 3 * sizeof *template);
    > if (ptr == NULL) { exit(1); }
    > template = ptr;
    > temp_ptr = ptr + 2;
    > *temp_ptr = malloc(5);
    > if (*temp_ptr == NULL) { exit(1); }
    > strncpy(*temp_ptr, "wxyz", 5);
    >
    > printf("%s\n%s\n%s\n", *template, *(template+1), *(template+2));
    >
    > return 0;


    It is not much of a critique rather a mention of my preference.
    I prefer to use array indexing instead of pointer offsets in my
    code. To me it makes the code easily understood it you code with
    indexing.

    However, your code is valid C, will compile and run as you expect.

    At this point, you might consider expanding your understanding of
    C by using some of the very good features of the language. I am
    referring to encapsulating of data in structs and the use of
    functions to do specific tasks. Your program of dynamically
    allocating an array of strings would be a good example of using
    these features of the language. You can make a struct that has
    as members a pointer to the array and a counter to keep accurate
    the number of elements(strings) in the array. Then you can write
    functions to manage the struct.

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

    typedef struct ARRAY
    {
    char **name;
    unsigned count;
    } ARRAY;

    char *addARRAY(ARRAY *p, const char *name);
    void freeARRAY(ARRAY *p);
    void printARRAY(const ARRAY *p);

    int main(void)
    {
    ARRAY presidents = {NULL, 0U}; /* initialize to begin */

    addARRAY(&presidents,"George Washington");
    addARRAY(&presidents,"Abe Lincoln");
    addARRAY(&presidents,"Richard Nixon");
    puts("\tTHE LISTED PRESIDENTS\n");
    printARRAY(&presidents);
    freeARRAY(&presidents);
    return 0;
    }

    char *addARRAY(ARRAY *p, const char *name)
    {
    char **tmp;
    unsigned i = p->count;

    if((tmp = realloc(p->name,(i + 1U)*sizeof(*p->name))) == NULL)
    return NULL;
    p->name = tmp;
    if((p->name = malloc(strlen(name)+1U)) == NULL)
    return NULL;
    strcpy(p->name,name);
    p->count++;
    return p->name;
    }

    void freeARRAY(ARRAY *p)
    {
    unsigned i;

    for(i = 0U;i < p->count;i++)
    free(p->name);
    free(p->name);
    p->name = NULL;
    p->count = 0U;
    }

    void printARRAY(const ARRAY *p)
    {
    unsigned i;

    for(i = 0U; i < p->count;i++)
    puts(p->name);
    return;
    }

    --
    Al Bowers
    Tampa, Fl USA
    mailto: (remove the x)
    http://www.geocities.com/abowers822/
     
    Al Bowers, Jul 15, 2003
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Bren
    Replies:
    8
    Views:
    2,047
    Stephen Howe
    Sep 4, 2003
  2. DrBob
    Replies:
    2
    Views:
    590
    Unforgiven
    Nov 26, 2003
  3. Jun Woong

    Re: Override malloc,calloc,realloc and free?

    Jun Woong, Jun 26, 2003, in forum: C Programming
    Replies:
    0
    Views:
    1,093
    Jun Woong
    Jun 26, 2003
  4. Dan Pop
    Replies:
    0
    Views:
    910
    Dan Pop
    Jun 26, 2003
  5. Douglas A. Gwyn

    Re: Override malloc,calloc,realloc and free?

    Douglas A. Gwyn, Jun 26, 2003, in forum: C Programming
    Replies:
    0
    Views:
    757
    Douglas A. Gwyn
    Jun 26, 2003
Loading...

Share This Page