[comp.lang.c] realloc questions

pcb@gator.cacs.usl.edu (Peter C. Bahrs) (02/27/90)

This may be a naive discussion but here goes...

I want to create an N (2 for now) dimensional datum of long integers.

    typedef struct a 
            { int row, col; long **arr} A;
    A *temp;

    temp=(A *) calloc (1, sizeof(A));  /* then initialize row and col ... */
    A->arr=(long *)calloc (A->row, sizeof(long *));
    for (j=0;j<A->row;j++)
       A->arr[j] = calloc(col,sizeof(long));

1) I should now be able to index as A->arr[j][k], correct?
2) or should type A contain long *arr[] instead?

Now I want to redimension the type to a new_row and new_col. 

   A->arr=(long *) realloc ((char *)&A->arr, new_row*sizeof(long *));
   for (j=0;j<A->new_row;j++)
       A->arr[j] = realloc(new_col,sizeof(long));
  
3) This doesn't work (sometimes).  I understand realloc will retain values
   of the minimum of the old and new dimensions, and 'frees' memory if
   the new is smaller.  
   
Any suggestions? 

/*----------- Thanks in advance... --------------------------------------+
| Peter C. Bahrs                                                         |
| The USL-NASA Project                                                   |
| Center For Advanced Computer Studies      INET: pcb@gator.cacs.usl.edu |
| 2 Rex Street                                                           |
| University of Southwestern Louisiana      ...!uunet!dalsqnt!gator!pcb  | 
| Lafayette, LA 70504                                                    |
+-----------------------------------------------------------------------*/

ping@cubmol.bio.columbia.edu (Shiping Zhang) (02/28/90)

In article <4563@rouge.usl.edu> pcb@gator.cacs.usl.edu (Peter C. Bahrs) writes:
>This may be a naive discussion but here goes...
>
>I want to create an N (2 for now) dimensional datum of long integers.
>
>    typedef struct a 
>            { int row, col; long **arr} A;
>    A *temp;

    Why don't you declare temp as varible A instead of a point to A,
    and omit the following line of code to allocate space for temp?

>    temp=(A *) calloc (1, sizeof(A));  /* then initialize row and col ... */

>    A->arr=(long *)calloc (A->row, sizeof(long *));
     ^       ^^^^^^         ^

    A (and also in following lines) should be temp and the cast
     should be (long **).

>    for (j=0;j<A->row;j++)
>       A->arr[j] = calloc(col,sizeof(long));
                   ^
    Need cast (long *).
 
>1) I should now be able to index as A->arr[j][k], correct?

    Yes, with the corrections.

>2) or should type A contain long *arr[] instead?
    
    No.  "long *arr[]" is illegal code in C.
 
>Now I want to redimension the type to a new_row and new_col. 
>
>   A->arr=(long *) realloc ((char *)&A->arr, new_row*sizeof(long *));
                ^                    ^
    
  The cast should (long **) and the address operator should NOT be used.
    
>   for (j=0;j<A->new_row;j++)
>       A->arr[j] = realloc(new_col,sizeof(long));
                   ^        ^
    Need cast.

>3) This doesn't work (sometimes).  I understand realloc will retain values
   Not surprising.  With the corrections, it should work all the time.

-ping

wkaufman@oracle.oracle.com (William P. Kaufman) (03/02/90)

Just one more thing to add to the discussion,...

To remind you (I added the previous corrections someone suggested to the code):

In article <4563@rouge.usl.edu> pcb@gator.cacs.usl.edu (Peter C. Bahrs) writes:
>
>    typedef struct a 
>            { int row, col; long **arr} A;
>    A *temp;
>
>    temp=(A *) calloc (1, sizeof(A));  /* then initialize row and col ... */
>    temp->arr=(long *)calloc (temp->row, sizeof(long *));
>    for (j=0;j<temp->row;j++)
>       temp->arr[j] = calloc(col,sizeof(long));
>
>1) I should now be able to index as temp->arr[j][k], correct?
>2) or should type A contain long *arr[] instead?

	That would be fine as long as you know the number of rows
ahead of time, as in "long *arr[30]", or some such.  Otherwise, "type
var[]" is only good if the variable is actually declared somewhere
else--say in an extern definition, or as a function parameter.  The
compiler needs to know at compilation time how large the objects are.
	As it is, it's just fine.

>Now I want to redimension the type to a new_row and new_col. 
>
>   temp->arr=(long *) realloc ((char *)&temp->arr, new_row*sizeof(long *));
>   for (j=0;j<temp->new_row;j++)
>       temp->arr[j] = realloc(new_col,sizeof(long));

	AUUGGGHHH!!  Sorry, I just recently had to re-write someone's
code that did this sort of thing.  You're re-allocating temp->arr
before you re-allocate temp->arr[j]! Just switch the order of the
realloc() calls and you should be fine.

	Happy hunting!

				-Bill Kaufman
				Nowhere you can get to by e-mail

moraes@cs.toronto.edu (Mark Moraes) (03/02/90)

Sigh.  After two incorrect/partial answers, I couldn't resist.

When posting code fragments that you have problems with, it might be a
good idea to make sure they compile first.  Also, please pass them
through either lint or an ANSI C compiler with good warnings and
prototyped header files.  (If your C environment has neither, then
you're working in an unsupportive environment and are asking for
trouble -- lint is a good friend.  People who don't realize how much
of a friend it can be should see refs 1 and 2 below)

The code posted in <4563@rouge.usl.edu> a) wouldn't compile
b) produced several serious lint errors (marked with ? and annotating
comments below) c) contained at least one serious logical error.  A
good example of how people seem to be confused about C pointers,
arrays, malloc and realloc.

    typedef struct a 
            { int row, col; long **arr} A;
    A *temp;

    temp=(A *) calloc (1, sizeof(A));  /* then initialize row and col ... */
    A->arr=(long *)calloc (A->row, sizeof(long *));
                 ?  /* cast to long **, not (long *) */
    for (j=0;j<A->row;j++)
       A->arr[j] = calloc(col,sizeof(long));

   A->arr=(long *) realloc ((char *)&A->arr, new_row*sizeof(long *));
               ?                    ? /* No need for & */
   for (j=0;j<A->new_row;j++)
                 ???????  /* 
			   * The pointers in temp->arr[] from row to
			   * new_row will contain undefined values,
			   * since realloc(temp->arr) does not zero the
			   * grown space.  All realloc guarantees is
			   * that the first MIN(old size, new size)
			   * bytes will contain exactly what they did
			   * before the realloc. Also, many reallocs do not
			   * guarantee ANSI C semantics of realloc(NULL,
			   * n) => malloc(n)
			   */
       A->arr[j] = realloc(new_col,sizeof(long));
                           ???????  /* Ouch!  What are you reallocing? */

The following is a version that at least passes a C compiler and lint
cleanly (modulo usual warnings about pointer alignment) -- it should
work. Run through cb -s for good measure.  (To keep it reasonably
short, I've committed the heinous "Can't happen" sin of not checking
the calloc/realloc error returns.  Any self respecting production
program would check those, of course)

typedef char * pointer;	    /* void * in ANSI C */

extern pointer calloc();
extern pointer realloc();
extern void free();	    /* returns int in older implementations */

typedef struct {
    unsigned int row, col;
    long **arr;
} A;

int
main()
{
    A *temp;
    unsigned int new_col, new_row;
    int j;

    temp = (A *) calloc ((unsigned) 1, sizeof(A));
    /* then initialize row and col ... */
    temp->row = 5;
    temp->col = 10;
    temp->arr = (long **) calloc (temp->row, sizeof(long *));
    for (j = 0; j < temp->row; j++)
       temp->arr[j] = (long *) calloc(temp->col, sizeof(long));

    /*
     * now you can access temp->arr[j][k] for 0 <= j < temp->row and 0
     * <= k < temp->col.
     */
    new_row = 10;
    new_col = 20;
    /* grow vector of pointers to rows */
    temp->arr = (long **) realloc ((pointer)temp->arr,
				   new_row * sizeof(long *));
    /*
     * grow old rows -- if new_row could be less than temp->row, we'd
     * have a to be a little more careful
     */
    for (j = 0; j < temp->row; j++)
	temp->arr[j] = (long *) realloc((pointer) temp->arr[j],
					new_col * sizeof(long));
    /* allocate new rows */
    for(j = temp->row; j < new_row; j++)
	temp->arr[j] = (long *) calloc(new_col, sizeof(long));

    /* 
     * just as a tutorial (xref the Turbo C malloc thread), we free the
     * arrays.  Remember the simple rule -- if you didn't get the
     * pointer from malloc/calloc or realloc, you CANNOT free it.
     */
    for(j = 0; j < new_row; j++)
	free((pointer) temp->arr[j]);
    free((pointer) temp->arr);
    free((pointer) temp);
    return 0;
}

Refs:

1. Ian Darwin and Geoff Collyer, "Can't Happen or /* NOTREACHED */ or
Real Programs Dump Core" USENIX Association Winter Conference, Dallas
1985 Proceedings.

2. Ian Darwin, "Checking C Programs with lint", O'Reilly Books.

msb@sq.sq.com (Mark Brader) (03/06/90)

[Karl Heuer: if you see this, let me know.]
[Anyone: if you see this twice, my apologies.]

> >   temp->arr=(long *) realloc ((char *)&temp->arr, new_row*sizeof(long *));
> >   for (j=0;j<temp->new_row;j++)
> >       temp->arr[j] = realloc(new_col,sizeof(long));

> You're re-allocating temp->arr before you re-allocate temp->arr[j]!
> Just switch the order of the realloc() calls and you should be fine.

No, it's more complicated than that.   You only want to use realloc() on
only those rows that existed and will continue to exist.  If there
are new rows, you have to malloc() them.  (Well, there are other ways
to arrange it, but they aren't any easier.)  Conversely, for the ones
that are ceasing to exist, you want to free() them.  Also, the first
call to realloc() is has an extra & and its result is wrongly cast, and
the second call is worse, having the arguments of calloc() instead of
realloc().  And temp->new_row is also wrong.

(Please let your compilers get the syntax errors out of your code before
you post it!  The code below has been compiled and (lightly) tested.)

Try this:

	    /* if number of rows is about to shrink, get rid of old ones
	     * (else, the loop will iterate 0 times) */
	for (j = new_row; j < temp->row; ++j)
		free ((char *) temp->arr[j]);

	    /* adjust number of rows */
	temp->arr = (long **) realloc ((char *) temp->arr,
						new_row * sizeof (long *));

	    /* if number of rows has just grown, allocate new ones
	     * (else, the loop will iterate 0 times) */
	for (j = temp->row; j < new_row; ++j)
		temp->arr[j] = (long *) malloc (new_col * sizeof (long));

	    /* adjust length of rows that exist and will continue to exist
	     * (while the three operations above must be in the order shown,
	     * this one could go at any point above or below them) */
	for (j = 0; j < temp->row && j < new_row; ++j)
		temp->arr[j] = (long *) realloc ((char *) temp->arr[j],
						new_col * sizeof (long));

	    /* update row and column information */
	temp->row = new_row;
	temp->col = new_col;

Another approach is to just free the whole structure and start over.
Since the original posting used calloc(), not malloc(), to allocate
the space, it may have been important that the array be initialized
so zeroes.  If realloc() enlarges the allocated space, the added
space is uninitialized.  In the above, I have elected to assume that
no initialization was really desired, and used malloc() and realloc().
If initialization IS really desired, the malloc(a*b) call should be
changed to calloc(a,b), and the last for-loop should become:

	for (j = 0; j < temp->row && j < new_row; ++j) {
		register int k;
		temp->arr[j] = (long *) realloc ((char *) temp->arr[j],
						new_col * sizeof (long));
		for (k = temp->col; k < new_col; ++k)
			temp->arr[j][k] = 0;
	}

or the equivalent thereof.

Depending on how your malloc() works, starting over may give better
results anyway, by maximizing malloc()'s chances to coalesce the
allocated chunks of memory.  Then again, it may not.


-- 
Mark Brader		     "I always pass on good advice.  It's the only thing
SoftQuad Inc., Toronto	      to do with it.  It is never any use to oneself."
utzoo!sq!msb, msb@sq.com 	-- Lord Goring  (Oscar Wilde: An Ideal Husband)

This article is in the public domain.