Velocity Reviews > increment and loop error

# increment and loop error

barry
Guest
Posts: n/a

 06-09-2010
Hi,

I have a problem that and the answer eludes me:

I am converting a 2D array to a 1D array within a for loop that looks
something like this:

ii=0;
for (i = 0; i < 300; i++) {
for (j = 0; j < 250; j++) {
oneD1[ii] = TwoD1[i][j];
oneD2[ii] = TwoD2[i][j];
oneD3[ii] = TwoD3[i][j];
ii++;

}
}

I get a segmentation fault and when I print out ii, the values exceeds
the product of 300*250.

Why does this happen when ii starts at zero and increments with 1 after
use? Even if I change ii++ to ii += 1, the same thing happens.

bart.c
Guest
Posts: n/a

 06-09-2010

"barry" <(E-Mail Removed)> wrote in message
news:huolnj\$4l5\$(E-Mail Removed)...
> Hi,
>
> I have a problem that and the answer eludes me:
>
> I am converting a 2D array to a 1D array within a for loop that looks
> something like this:
>
> ii=0;
> for (i = 0; i < 300; i++) {
> for (j = 0; j < 250; j++) {
> oneD1[ii] = TwoD1[i][j];
> oneD2[ii] = TwoD2[i][j];
> oneD3[ii] = TwoD3[i][j];
> ii++;
>
> }
> }
>
> I get a segmentation fault and when I print out ii, the values exceeds
> the product of 300*250.

What's the actual value printed? I get 75000 at the end (but without any
assignments done). Try printing i,j and ii on each iteration, but with
limits of 30 and 25 instead; does ii now exceed 750 at the end?

How big are oneD1 etc?

--
Bartc

Seebs
Guest
Posts: n/a

 06-09-2010
On 2010-06-09, barry <(E-Mail Removed)> wrote:
> I have a problem that and the answer eludes me:

> I am converting a 2D array to a 1D array within a for loop that looks
> something like this:

Okay.

> ii=0;
> for (i = 0; i < 300; i++) {
> for (j = 0; j < 250; j++) {
> oneD1[ii] = TwoD1[i][j];
> oneD2[ii] = TwoD2[i][j];
> oneD3[ii] = TwoD3[i][j];
> ii++;
>
> }
> }

> I get a segmentation fault and when I print out ii, the values exceeds
> the product of 300*250.

Hmm.

> Why does this happen when ii starts at zero and increments with 1 after
> use? Even if I change ii++ to ii += 1, the same thing happens.

It happens because of something in the code you didn't show us, such as
the code declaring and initializing the arrays. For instance, if one
of your one-dimensional arrays was not big enough, you could be scrambling
other stuff.

#include <stdio.h>

int oneD1[250*300];
int oneD2[250*300];
int oneD3[250*300];
int twoD1[250][300];
int twoD2[250][300];
int twoD3[250][300];

int
main(void) {
int i, j;
int ii = 0;
for (i = 0; i < 250; ++i) {
for (j = 0; j < 300; ++j) {
oneD1[ii] = twoD1[i][j];
oneD2[ii] = twoD2[i][j];
oneD3[ii] = twoD3[i][j];
++ii;
}
}
printf("%d\n", ii);
return 0;
}

This works exactly as expected, and prints "75000". Strip your program
down similarly and you'll find the problem or reduce it to something about
this size in which we can tell you what the problem is

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / http://www.velocityreviews.com/forums/(E-Mail Removed)
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!

John Bode
Guest
Posts: n/a

 06-09-2010
On Jun 9, 1:15*pm, barry <(E-Mail Removed)> wrote:
> Hi,
>
> I have a problem that and the answer eludes me:
>
> I am converting a 2D array to a 1D array within a for loop that looks
> something like this:
>
> ii=0;
> for (i = 0; i < 300; i++) {
> * * for (j = 0; j < 250; j++) {
> * * * * *oneD1[ii] = TwoD1[i][j];
> * * * * *oneD2[ii] = TwoD2[i][j];
> * * * * *oneD3[ii] = TwoD3[i][j];
> * * * * *ii++;
>
> }
> }
>
> I get a segmentation fault and when I print out ii, the values exceeds
> the product of 300*250.
>
> Why does this happen when ii starts at zero and increments with 1 after
> use? Even if I change ii++ to ii += 1, the same thing happens.
>

I'm going to guess that ii is a 16-bit signed int, which is not wide
enough to hold the result of 300*250 (75000 > 65535). What's likely
happening is that you're getting a singed integer overflow once you
attempt to increment past 65535, and when you try to use the overflow
value to index the array, you get the segfault.

You need to declare ii as either an *unsigned* 16-bit integer or a
wider signed type (such as long). Personally, I declare anything
that's going to be used as an array index as size_t, no matter how big
the array is. Just one less thing to worry about.

Keith Thompson
Guest
Posts: n/a

 06-09-2010
barry <(E-Mail Removed)> writes:
> I have a problem that and the answer eludes me:
>
> I am converting a 2D array to a 1D array within a for loop that looks
> something like this:
>
> ii=0;
> for (i = 0; i < 300; i++) {
> for (j = 0; j < 250; j++) {
> oneD1[ii] = TwoD1[i][j];
> oneD2[ii] = TwoD2[i][j];
> oneD3[ii] = TwoD3[i][j];
> ii++;
>
> }
> }
>
> I get a segmentation fault and when I print out ii, the values exceeds
> the product of 300*250.
>
> Why does this happen when ii starts at zero and increments with 1 after
> use? Even if I change ii++ to ii += 1, the same thing happens.

When I compile and execute your code (with ii, i, and j declared
as int and the array element assignments commented out), I don't
see the problem you're describing.

Your program would misbehave if int is 16 bits (and if ii is declared
as int), since 300*250 is 75000, which exceeds the maximum value of
a 16-bit int, but that wouldn't cause the problem you're describing.

Among other things, you haven't shown us the declarations of any
of the 8 variables you refer to, or the code you use to display
the value of ii.

In reducing your code to a minimal example, you appear to have
removed whatever caused the problem you're observing. Post a small,
complete, compilable, executable program that clearly exhibits the
problem, and we can probably help you. (It's also fairly likely that
you'll solve the problem yourself while creating such a test case.)
Copy-and-paste the exact program source and its output; don't just
summarize or describe either.

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

Ben Bacarisse
Guest
Posts: n/a

 06-09-2010
John Bode <(E-Mail Removed)> writes:

> On Jun 9, 1:15Â*pm, barry <(E-Mail Removed)> wrote:

<snip>
>> ii=0;
>> for (i = 0; i < 300; i++) {
>> Â* Â* for (j = 0; j < 250; j++) {
>> Â* Â* Â* Â* Â*oneD1[ii] = TwoD1[i][j];
>> Â* Â* Â* Â* Â*oneD2[ii] = TwoD2[i][j];
>> Â* Â* Â* Â* Â*oneD3[ii] = TwoD3[i][j];
>> Â* Â* Â* Â* Â*ii++;
>> }
>> }
>>
>> I get a segmentation fault and when I print out ii, the values exceeds
>> the product of 300*250.

<snip>
> I'm going to guess that ii is a 16-bit signed int, which is not wide
> enough to hold the result of 300*250 (75000 > 65535).

Nit: I think you mean 32767 here.

<snip>
> You need to declare ii as either an *unsigned* 16-bit integer or a
> wider signed type (such as long).

Using unsigned won't work. It may stop the seg. fault (by never being
negative) but the code will go wrong in some other way which may be even
harder to find.

> Personally, I declare anything
> that's going to be used as an array index as size_t, no matter how big
> the array is. Just one less thing to worry about.

--
Ben.

barry
Guest
Posts: n/a

 06-09-2010
The actual .c file is over 8000 lines long and I didn't think it was
appropriate to post it all. The function IS this:

static int TwoDToOneD(void) {

int i,j, ii;

MALLOC(ret.lightcon, sizeof(float*)*AreaRow/res*AreaCol/res);
MALLOC(ret.valconcen, sizeof(int*)*AreaRow/res*AreaCol/res);
MALLOC(ret.bias100, sizeof(float*)*AreaRow/res*AreaCol/res);
MALLOC(ret.bias75, sizeof(float*)*AreaRow/res*AreaCol/res);
MALLOC(ret.bias50, sizeof(float*)*AreaRow/res*AreaCol/res);
MALLOC(ret.bias25, sizeof(float*)*AreaRow/res*AreaCol/res);
MALLOC(ret.latitude, sizeof(float*)*AreaRow/res*AreaCol/res);
MALLOC(ret.longitude, sizeof(float*)*AreaRow/res*AreaCol/res);

ii = 0;
printf("Row: %d\tCol: %d\n", AreaRow/res, AreaCol/res);
for (i = 0; i < AreaRow/res; i++) {
for (j = 0; j < AreaCol/res; j++) {
printf("ii: %d\ti: %d\tj: %d\n", ii, i, j);
ret.lightcon[ii] = work.lightcon2D[i][j];
ret.bias100[ii] = work.bias1002D[i][j];
ret.bias75[ii] = work.bias752D[i][j];
ret.bias50[ii] = work.bias502D[i][j];
ret.bias25[ii] = work.bias252D[i][j];
ret.latitude[ii] = work.latitude2D[i][j];
ret.longitude[ii] = work.longitude2D[i][j];
ret.valconcen[ii] = work.valconcen2D[i][j];
ii++;
}
}
for (i = 0; i < AreaCol; i++) {
free(work.lightcon2D[i]);
free(work.bias1002D[i]);
free(work.bias752D[i]);
free(work.bias502D[i]);
free(work.bias252D[i]);
free(work.latitude2D[i]);
free(work.longitude2D[i]);
free(work.valconcen2D[i]);
}
free(work.lightcon2D);
free(work.bias1002D);
free(work.bias752D);
free(work.bias502D);
free(work.bias252D);
free(work.latitude2D);
free(work.longitude2D);
free(work.valconcen2D);

return 1;
}

Keith Thompson wrote:
> barry <(E-Mail Removed)> writes:
>> I have a problem that and the answer eludes me:
>>
>> I am converting a 2D array to a 1D array within a for loop that looks
>> something like this:
>>
>> ii=0;
>> for (i = 0; i < 300; i++) {
>> for (j = 0; j < 250; j++) {
>> oneD1[ii] = TwoD1[i][j];
>> oneD2[ii] = TwoD2[i][j];
>> oneD3[ii] = TwoD3[i][j];
>> ii++;
>>
>> }
>> }
>>
>> I get a segmentation fault and when I print out ii, the values exceeds
>> the product of 300*250.
>>
>> Why does this happen when ii starts at zero and increments with 1 after
>> use? Even if I change ii++ to ii += 1, the same thing happens.

>
> When I compile and execute your code (with ii, i, and j declared as int
> and the array element assignments commented out), I don't see the
> problem you're describing.
>
> Your program would misbehave if int is 16 bits (and if ii is declared as
> int), since 300*250 is 75000, which exceeds the maximum value of a
> 16-bit int, but that wouldn't cause the problem you're describing.
>
> Among other things, you haven't shown us the declarations of any of the
> 8 variables you refer to, or the code you use to display the value of
> ii.
>
> In reducing your code to a minimal example, you appear to have removed
> whatever caused the problem you're observing. Post a small, complete,
> compilable, executable program that clearly exhibits the problem, and we
> can probably help you. (It's also fairly likely that you'll solve the
> problem yourself while creating such a test case.) Copy-and-paste the
> exact program source and its output; don't just summarize or describe
> either.

Keith Thompson
Guest
Posts: n/a

 06-09-2010
barry <(E-Mail Removed)> writes:
> Keith Thompson wrote:

[...]
>> Among other things, you haven't shown us the declarations of any of the
>> 8 variables you refer to, or the code you use to display the value of
>> ii.
>>
>> In reducing your code to a minimal example, you appear to have removed
>> whatever caused the problem you're observing. Post a small, complete,
>> compilable, executable program that clearly exhibits the problem, and we
>> can probably help you. (It's also fairly likely that you'll solve the
>> problem yourself while creating such a test case.) Copy-and-paste the
>> exact program source and its output; don't just summarize or describe
>> either.

>
> The actual .c file is over 8000 lines long and I didn't think it was
> appropriate to post it all.

You're right, that would be too long.

> The function IS this:
>
> static int TwoDToOneD(void) {
>
> int i,j, ii;
>
> MALLOC(ret.lightcon, sizeof(float*)*AreaRow/res*AreaCol/res);

[7 lines deleted]
>
> ii = 0;
> printf("Row: %d\tCol: %d\n", AreaRow/res, AreaCol/res);
> for (i = 0; i < AreaRow/res; i++) {
> for (j = 0; j < AreaCol/res; j++) {
> printf("ii: %d\ti: %d\tj: %d\n", ii, i, j);
> ret.lightcon[ii] = work.lightcon2D[i][j];

[7 lines deleted]
> ii++;
> }
> }
> for (i = 0; i < AreaCol; i++) {
> free(work.lightcon2D[i]);

[7 lines deleted]
> }
> free(work.lightcon2D);

[7 lines deleted]
>
> return 1;
> }

You need to do some work here. If you can pare down your code to
something *small* and *self-contained* that clearly exhibits the
problem, we can probably help. The code you posted still refers to
a blortload of variables whose declarations you haven't shown us.
We don't know how MALLOC (presumably a macro) is defined. I haven't
studied the code in depth, but it's still likely that you haven't
shown us the code that's actually causing the problem.

You're processing 8 different sets of arrays; pare that down to 1.
Don't worry about the pared-down program being functionally useful;
just do something that clearly misbehaves in the same way as your
original program. And so forth.

Also, please don't top-post (I've corrected it here). See:

http://www.caliburn.nl/topposting.html
http://www.cpax.org.uk/prg/writings/topposting.php

--
Keith Thompson (The_Other_Keith) (E-Mail Removed) <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

bart.c
Guest
Posts: n/a

 06-09-2010
"barry" <(E-Mail Removed)> wrote in message
news:huoq2q\$bva\$(E-Mail Removed)...

> int i,j, ii;

> ii = 0;
> printf("Row: %d\tCol: %d\n", AreaRow/res, AreaCol/res);
> for (i = 0; i < AreaRow/res; i++) {
> for (j = 0; j < AreaCol/res; j++) {
> printf("ii: %d\ti: %d\tj: %d\n", ii, i, j);

Perhaps extend that to:
printf("ii: %d\ti: %d\tj: %d\tMax: %d\n", ii, i,
j,AreaRow/res*AreaCol/res);

What are i and j doing when ii exceeds that total? What does ii reach when
the loops finally terminate?

(Comment out the assignments temporarily.)

--
Bartc

Joachim Schipper
Guest
Posts: n/a

 06-09-2010
barry <(E-Mail Removed)> wrote:
> The actual .c file is over 8000 lines long and I didn't think it was
> appropriate to post it all. The function IS this:
>
> static int TwoDToOneD(void) {
>
> int i,j, ii;
>
> MALLOC(ret.lightcon, sizeof(float*)*AreaRow/res*AreaCol/res);
> MALLOC(ret.valconcen, sizeof(int*)*AreaRow/res*AreaCol/res);

....
> MALLOC(ret.longitude, sizeof(float*)*AreaRow/res*AreaCol/res);
>
> ii = 0;
> printf("Row: %d\tCol: %d\n", AreaRow/res, AreaCol/res);
> for (i = 0; i < AreaRow/res; i++) {

^^^^^^^^^^^^^^^
> for (j = 0; j < AreaCol/res; j++) {
> printf("ii: %d\ti: %d\tj: %d\n", ii, i, j);
> ret.lightcon[ii] = work.lightcon2D[i][j];
> ret.bias100[ii] = work.bias1002D[i][j];

....
> ret.valconcen[ii] = work.valconcen2D[i][j];
> ii++;
> }
> }
> for (i = 0; i < AreaCol; i++) {

^^^^^^^^^^^
> free(work.lightcon2D[i]);
> free(work.bias1002D[i]);

....
> free(work.valconcen2D[i]);
> }
> free(work.lightcon2D);
> free(work.bias1002D);

....
> free(work.valconcen2D);
>
> return 1;
> }

This doesn't really make sense, does it? Fix that first.

As an aside, two-dimensional arrays (of size x by y) are best malloc()d
as follows (with error checking to taste):

int **p, i;
p = malloc(x * sizeof(*p));
p[0] = malloc(x * y * sizeof(**p));
for (i = 1; i < x; i++)
p[i] = p[0] + i * y;

and free()d as follows:

free(p[0]);
free(p);

this reduces malloc() overhead, both in runtime and memory use,
significantly.

Joachim