2D Array not working

[=?ISO-8859-1?Q?Jonathan_Gro=DF?=]

Hi everybody,

in the sample below my programm gives a "Bus error" on the marked line.
I don't get it why that happens. I followed the instructions in the
C-FAQ.

#include <math.h>
#include <stdlib.h>

#define dim 2
#define length 3

int main (int argc, const char * argv[]) {
int i,j;
int volume=(int)pow(length,dim);

int **nbrs = (int **) malloc(volume * sizeof(int *));
for (i=0; i<volume; i++)
*(nbrs+i) = (int *) malloc(2*dim * sizeof(int));

for (i=0;i<volume;i++)
for(j=0;j<(2*dim);j++) {
*(nbrs+i)=i;
*(*(nbrs+i)+j)=j; //here is the problem
}

for (i=0; i<volume; i++)
printf("%d\n", nbrs[i]);

return 0;
}

[mark_bluemel]

On 5 Feb, 15:28, Jonathan Groß <[e-mail]>
wrote:

Hi everybody,

in the sample below my programm gives a "Bus error" on the marked line.
I don't get it why that happens. I followed the instructions in the
C-FAQ.

God only knows what you are trying to achieve here. It would be nice
to have a short description...

#include <math.h>
#include <stdlib.h>

#include <stdio.h /* so we've got the printf functions prototype
in scope */

#define dim 2
#define length 3

int main (int argc, const char * argv[]) {
int i,j;
int volume=(int)pow(length,dim);

On this occasion, you got away with it, but do you believe this will
always work?

int **nbrs = (int **) malloc(volume * sizeof(int *));
for (i=0; i<volume; i++)
*(nbrs+i) = (int *) malloc(2*dim * sizeof(int));

So you've filled the first array with pointers to storage...

for (i=0;i<volume;i++)
for(j=0;j<(2*dim);j++) {
*(nbrs+i)=i;

So now you repeatedly overwrite those pointers with an integer value.

*(*(nbrs+i)+j)=j; //here is the problem

And expect to dereference that value... (BTW, please use /*...*/ for
comments as they are much better handled when cut and pasted from
newgroup articles).

}

for (i=0; i<volume; i++)
printf("%d\n", nbrs[i]);

return 0;

}

[Ben Bacarisse]

Jonathan Groß <pg******@studserv.uni-leipzig.dewrites:

Hi everybody,

in the sample below my programm gives a "Bus error" on the marked
line.

You either need to turn up the warning level on your compiler or you
need to listen to it! You destroy a correctly set up pointer by
assigning an integer value to it. Trouble is inevitable after that.

BTW, you don't have a 2D array. You have an array of pointers to
arrays of integers. C does have 2D arrays and they work differently.

I don't get it why that happens. I followed the instructions in
the C-FAQ.

#include <math.h>
#include <stdlib.h>

You need <stdio.hfor printf. Did your compiler not tell you this?

#define dim 2
#define length 3

int main (int argc, const char * argv[]) {
int i,j;
int volume=(int)pow(length,dim);

int **nbrs = (int **) malloc(volume * sizeof(int *));

There is no need to cast the return value from malloc. The preferred
idiom is:

int **nbrs = malloc(volume * sizeof *nbrs);

for (i=0; i<volume; i++)
*(nbrs+i) = (int *) malloc(2*dim * sizeof(int));

I would write:
nbrs[i] = malloc(2 * dim * sizeof *nbrs[i]);

Array access is simpler using [] rather than * and +.

>
for (i=0;i<volume;i++)
for(j=0;j<(2*dim);j++) {
*(nbrs+i)=i;

Your problem is really here, not on the line below. *(nbrs+i) is the
same as nbrs[i] which is a pointer to some ints. You correctly set it
up a few lines above. Putting i into it is illegal C. I can't
correct it because I have no idea what you intended to do here.

*(*(nbrs+i)+j)=j; //here is the problem

nbrs[i][j] = j is a much simpler way to write this.

}

for (i=0; i<volume; i++)
printf("%d\n", nbrs[i]);

%d needs and int. nbrs[i] is an int *. What did you want to do here?
Maybe you intended to have another loop and print nbrs[i][j].

return 0;
}

--
Ben.

[Flash Gordon]

Jonathan Groß wrote, On 05/02/07 15:28:

Hi everybody,

in the sample below my programm gives a "Bus error" on the marked line.
I don't get it why that happens. I followed the instructions in the C-FAQ.

Not very well though.

#include <math.h>
#include <stdlib.h>

#define dim 2
#define length 3

Macros are traditionally all upper case, going against this convention
makes it harder for those used to it, i.e. most C programmers.

int main (int argc, const char * argv[]) {
int i,j;
int volume=(int)pow(length,dim);

You don't need the cast, it will behave exactly the same without it.
Also, without a good reason I'm not convinced that using a floating
point function is the best way to generate an array size.

int **nbrs = (int **) malloc(volume * sizeof(int *));

Does the C FAQ really use all these horrible and needless casts.
<check/>
No, it doesn't. Why do the extra typing when it is just more to get
wrong and can even hide two different problems. Also, as the C FAQ notes
in a footnote, there is a better way to use sizeof.
int **nbrs = malloc(volume * sizeof *nbrs);
About 10 fewer characters to type and less error prone.

Also, as the FAQ notes, you should check the return value of malloc.

for (i=0; i<volume; i++)
*(nbrs+i) = (int *) malloc(2*dim * sizeof(int));

Same comments as before, aditionally I would use array is easier to read
(in my opinion) in this case.
nbrs[i] = malloc(2*dim * sizeof *nbrs[i]);

for (i=0;i<volume;i++)
for(j=0;j<(2*dim);j++) {
*(nbrs+i)=i;
*(*(nbrs+i)+j)=j; //here is the problem

Again, array notation makes it easier to read.
nbrs[i]=i;
nbrs[i][j]=j;

Now look at the first of these two lines and compare it to the loop
allocating the space for the rows. You should see that you are
overwriting all those pointers you were so careful to allocate. Your
compiler should even have complained about the assignment "*(nbrs+i)=i"
to let you know it is a problem. Whether the problem is that you want
something other than a 2D array or whether you don't understand pointers
to pointers or what is hard to say at this point.

}

for (i=0; i<volume; i++)
printf("%d\n", nbrs[i]);

This loop also does not make sense.

return 0;
}

--
Flash Gordon

[jaysome]

On Mon, 5 Feb 2007 16:28:18 +0100, Jonathan Groß
<pg******@studserv.uni-leipzig.dewrote:

>Hi everybody,

in the sample below my programm gives a "Bus error" on the marked line.
I don't get it why that happens. I followed the instructions in the
C-FAQ.

#include <math.h>
#include <stdlib.h>

#define dim 2
#define length 3

int main (int argc, const char * argv[]) {

Others have given you good advice about your problems.

I would add that you do not use either argc or argv in your code, so a
better definition to use for main() would be:

int main(void)

The C standard allows this definition, and maybe even yours [1]:

<quote>
5.1.2.2.1 Program startup

The function called at program startup is named main. The
implementation declares no prototype for this function. It shall be
defined with a return type of int and with no parameters:

int main(void) { /* ... */ }

or with two parameters (referred to here as argc and argv, though any
names may be used, as they are local to the function in which they are
declared):

int main(int argc, char *argv[]) { /* ... */ }

or equivalent; or in some other implementation-defined manner.
</quote>

[snip]

Best regards
--
jay

[1]
Does the C standard allow the OP's additional qualification of const
to the second argument to main()? In other words, is:

int main (int argc, const char * argv[])

an acceptable definition for main(), when, strictly speaking, it
should be:

int main(int argc, char *argv[])

I suspect so, since the former is basically more restrictive about
what you can "do" with the second argument, and poses no harm in
placing such a restriction.

[Jamie Boy]

Jonathan Groß:

Hi everybody,

in the sample below my programm gives a "Bus error" on the marked line.
I don't get it why that happens. I followed the instructions in the
C-FAQ.

#include <math.h>
#include <stdlib.h>

#define dim 2
#define length 3

Most people think it's a good idea to use capital letters for macro names.

int main (int argc, const char * argv[]) {
int i,j;
int volume=(int)pow(length,dim);

The cast is redundant from a Standard C point of view, but it may serve to
suppress a compiler warning.

It's also a good idea to use "const" whenever possible, e.g.:

int const volume = (int)pow(length,dim);

int **nbrs = (int **) malloc(volume * sizeof(int *));

This cast serves no purpose. In fact, it:

a) Makes the code look messy
b) Could potentially hide the error of neglecting to include stdlib.h

A better solution would be:

int **const nbrs = malloc(volume * sizeof*nbrs);

for (i=0; i<volume; i++)
*(nbrs+i) = (int *) malloc(2*dim * sizeof(int));

Replace *(nbrs+i) with nbrs[i], it's neater that way:

for(i=0;i!=volume;++i) nbrs[i] = malloc(2*dim*sizeof**nbrs);

Or alternatively, you could loop through it with a pointer:

int const **p = nbrs;
int const *const *const pover = nbrs + volume;

do *p++ = malloc(2*dim*sizeof**pp);
while (pover!=p);

for (i=0;i<volume;i++)
for(j=0;j<(2*dim);j++) {
*(nbrs+i)=i;
*(*(nbrs+i)+j)=j; //here is the problem
}

If the *(nbrs+i)=i; line compiles then you might wanna pick up another
compiler. You should get a type mismatch. The thing on the left is an int*,
while the thing on the right is an int.

You think you're working with a two-dimensional array (i.e. an array of
arrays), when in actual fact you're working with an array of pointers to
arrays.

for (i=0; i<volume; i++)
printf("%d\n", nbrs[i]);

return 0;
}

--
~/JB\~