From: msb@sq.sq.com (Mark Brader)
Subject: Re: Copying file of int to 2-d array
Organization: SoftQuad Inc., Toronto, Canada
Date: Sun, 30 Aug 92 20:22:46 GMT

> > >     array = (int *) malloc (98 * sizeof (int *));
> >       array[counter] = (int *) malloc (98 * sizeof (int *));

Actually, the second "int *" should be "int".  This bug wouldn't make
itself obvious unless you found a machine where int * uses fewer bytes
than int.  This is one reason that I recommend using sizeof object
rather than sizeof (type) in most cases; the line then becomes

          array[counter] = (int *) malloc (98 * sizeof *array[counter]);

The hardcoded constant 98 is of course also a bad idea.

> Sorry people, I did have the array[counter] in my program (it was a typo
> error when I retyped my program for the post).

If you post code, post the actual code you're talking about; don't retype
it.  If the example is too long to post, make a copy and edit away the
irrelevant parts (note: declarations are never irrelevant).  Or better
yet, construct a smaller program that has the same problem, verify that
it has it, and post the smaller version!

(I violate my own rule below; the statements I post have not been tested.
But these are isolated statements and not purportedly complete programs.)

> solved thanks to everyone.

So what was the real problem?


> However, I have an alternative query, after using the dynamic array, is
> it possible for me to free it with one command :-
> 
>    free (array)
> 
> or do I have to set a loop to free the array??

The rule is one call to free() for each call to malloc() or calloc().
So if you allocate it in the way you did, you need a loop.  If the total
size of the array is not too large, though, the loop can be avoided by
doing a single malloc() for the whole array, except the dope vector
which you malloc() first, and dealing off chunks of it for the dope
vector to point to.  One of several equivalent ways to code this is:

      array = (int **) malloc (98 * sizeof *array);
      array[0] = (int *) malloc (98 * 98 * sizeof *array[0]);
      for (counter = 1; counter < 98; ++counter)
         array[counter] = array[0] + 98*counter;

If the number of columns in the array is known at compile time, another
option is to use a pointer-to-array type and eliminate the dope vector.
Declare array as

      int (*array)[98];
and write
      array = (int (*)[98]) malloc (98 * sizeof *array);

and you are done.  The first two 98's are the number of columns, which
must be a constant expression, while the third one is the number of
rows -- see how hardcoded constants make the code hard to read?

However, both of these alternative solutions depend on the array size
not being too large.  The largest value that you can depend on passing
to malloc() on old implementations is 32767; values that won't fit in
an int can never be passed.  Even if you can pass the value to malloc(),
you may not be able to get a single chunk of memory that big, so these
single-malloc() methods are more prone to failure.

Failure of malloc() being, incidentally, something that you should
check for after every call to it.

>    for (counter = 0; counter < 98; ++counter)
>        free (array[counter];
>    free (array);

Modern implementations, based on ANSI C, do not require certain casts
that were required in older implementations.  Your code uses one of these
casts, but not another.  If portability to older implementations is not
an issue, then the cast of the value returned from malloc() is needless;
for example, you can write:

         array[counter] = malloc (98 * sizeof *array[counter]);

Whereas, if portability *is* an issue, or if you want to play safe about
that, or if you simply think that redundant casts are good style, then a
cast is equally needed on the value passed to free():

         free ((char *) array[counter]);

Modern and middle-aged implementations will also accept "void *" here
in place of "char *".

-- 
Mark Brader			"'You wanted it to WORK?  That costs EXTRA!'
SoftQuad Inc., Toronto		 is probably the second-place security hole
utzoo!sq!msb, msb@sq.com	 after simple carelessness."	-- John Woods

This article is in the public domain.
Newsgroups: comp.lang.c,comp.sys.sun.misc