Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > multiple realloc w/ malloc blows away data

Reply
Thread Tools

multiple realloc w/ malloc blows away data

 
 
WL
Guest
Posts: n/a
 
      07-14-2003
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...)
 
Reply With Quote
 
 
 
 
Juergen Heinzl
Guest
Posts: n/a
 
      07-14-2003
In article <bev6cr$957j7$(E-Mail Removed)-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 : http://www.velocityreviews.com/forums/(E-Mail Removed) \ send money instead /
 
Reply With Quote
 
 
 
 
Bertrand Mollinier Toublet
Guest
Posts: n/a
 
      07-14-2003
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

 
Reply With Quote
 
Eric Sosman
Guest
Posts: n/a
 
      07-14-2003
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.

--
(E-Mail Removed)
 
Reply With Quote
 
Al Bowers
Guest
Posts: n/a
 
      07-14-2003


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[i]);
free(template);
exit(EXIT_FAILURE);
}
template = temp_ptr;
if((template[count] = malloc(5)) == NULL)
{
for(i = 0; i < count;i++) free(template[i]);
free(template);
exit(EXIT_FAILURE);
}
strcpy(template[count],"wxyz");
count++;
for(i = 0;i < count;i++)
printf("template[%u] = \"%s\"\n",i,template[i]);

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



--
Al Bowers
Tampa, Fl USA
mailto: (E-Mail Removed) (remove the x)
http://www.geocities.com/abowers822/

 
Reply With Quote
 
Default User
Guest
Posts: n/a
 
      07-14-2003


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
 
Reply With Quote
 
Wenchi
Guest
Posts: n/a
 
      07-14-2003
Eric Sosman <(E-Mail Removed)> 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...)
 
Reply With Quote
 
Christian Bau
Guest
Posts: n/a
 
      07-15-2003
In article <beveb4$9g9sc$(E-Mail Removed)-berlin.de>,
Wenchi <(E-Mail Removed)> wrote:

> Eric Sosman <(E-Mail Removed)> 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. )
 
Reply With Quote
 
Blah
Guest
Posts: n/a
 
      07-15-2003

"Wenchi" <(E-Mail Removed)> 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>


 
Reply With Quote
 
Al Bowers
Guest
Posts: n/a
 
      07-15-2003


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[i] = malloc(strlen(name)+1U)) == NULL)
return NULL;
strcpy(p->name[i],name);
p->count++;
return p->name[i];
}

void freeARRAY(ARRAY *p)
{
unsigned i;

for(i = 0U;i < p->count;i++)
free(p->name[i]);
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[i]);
return;
}

--
Al Bowers
Tampa, Fl USA
mailto: (E-Mail Removed) (remove the x)
http://www.geocities.com/abowers822/

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Different sizes of data and function pointers on a machine -- void*return type of malloc, calloc, and realloc Myth__Buster C Programming 23 06-26-2012 08:29 PM
Big Pentax 645 blows away top DSLRs RichA Digital Photography 3 12-10-2010 08:34 AM
dpreview test: Leica Digilux 2 blows away Canon G5 ThomasH Digital Photography 12 05-15-2004 03:11 AM
A Question about new vs malloc and realloc. DrBob C++ 2 11-26-2003 10:17 PM
Problem with malloc, realloc, _msize and memcpy Bren C++ 8 09-03-2003 11:01 PM



Advertisments