# increment and loop error

Discussion in 'C Programming' started by barry, Jun 9, 2010.

1. ### barryGuest

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[j];
oneD2[ii] = TwoD2[j];
oneD3[ii] = TwoD3[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.

barry, Jun 9, 2010

2. ### bart.cGuest

"barry" <> wrote in message
news:huolnj\$4l5\$...
> 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[j];
> oneD2[ii] = TwoD2[j];
> oneD3[ii] = TwoD3[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

bart.c, Jun 9, 2010

3. ### SeebsGuest

On 2010-06-09, barry <> 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[j];
> oneD2[ii] = TwoD2[j];
> oneD3[ii] = TwoD3[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[j];
oneD2[ii] = twoD2[j];
oneD3[ii] = twoD3[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.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!

Seebs, Jun 9, 2010
4. ### John BodeGuest

On Jun 9, 1:15 pm, barry <> 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[j];
>          oneD2[ii] = TwoD2[j];
>          oneD3[ii] = TwoD3[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.
>
> Thanks in advance

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.

John Bode, Jun 9, 2010
5. ### Keith ThompsonGuest

barry <> 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[j];
> oneD2[ii] = TwoD2[j];
> oneD3[ii] = TwoD3[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) <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"

Keith Thompson, Jun 9, 2010
6. ### Ben BacarisseGuest

John Bode <> writes:

> On Jun 9, 1:15Â pm, barry <> wrote:

<snip>
>> ii=0;
>> for (i = 0; i < 300; i++) {
>> Â  Â  for (j = 0; j < 250; j++) {
>> Â  Â  Â  Â  Â oneD1[ii] = TwoD1[j];
>> Â  Â  Â  Â  Â oneD2[ii] = TwoD2[j];
>> Â  Â  Â  Â  Â oneD3[ii] = TwoD3[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.

Ben Bacarisse, Jun 9, 2010
7. ### barryGuest

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[j];
ret.bias100[ii] = work.bias1002D[j];
ret.bias75[ii] = work.bias752D[j];
ret.bias50[ii] = work.bias502D[j];
ret.bias25[ii] = work.bias252D[j];
ret.latitude[ii] = work.latitude2D[j];
ret.longitude[ii] = work.longitude2D[j];
ret.valconcen[ii] = work.valconcen2D[j];
ii++;
}
}
for (i = 0; i < AreaCol; 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);
}
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 <> 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[j];
>> oneD2[ii] = TwoD2[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]

>
> 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.

barry, Jun 9, 2010
8. ### Keith ThompsonGuest

barry <> 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[j];

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

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

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

That's still not very helpful.

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) <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"

Keith Thompson, Jun 9, 2010
9. ### bart.cGuest

"barry" <> wrote in message
news:huoq2q\$bva\$...

> 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

bart.c, Jun 9, 2010
10. ### Joachim SchipperGuest

barry <> 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[j];
> ret.bias100[ii] = work.bias1002D[j];

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

^^^^^^^^^^^
> free(work.lightcon2D);
> free(work.bias1002D);

....
> free(work.valconcen2D);
> }
> 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 = 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

Joachim Schipper, Jun 9, 2010
11. ### Ike NaarGuest

In article <huoq2q\$bva\$>, barry <> wrote:
> 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);

What is the element type of those arrays? If it is something other
than float* (especially if it is something that has a sizeof larger
than sizeof(float*), the allocated arrays will be the wrong size.
Looking at the names, I'd guess that the element types are not float*,
perhaps they are float or double?

> 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[j];
> ret.bias100[ii] = work.bias1002D[j];
> ret.bias75[ii] = work.bias752D[j];
> ret.bias50[ii] = work.bias502D[j];
> ret.bias25[ii] = work.bias252D[j];
> ret.latitude[ii] = work.latitude2D[j];
> ret.longitude[ii] = work.longitude2D[j];
> ret.valconcen[ii] = work.valconcen2D[j];
> ii++;
> }
> }

One thing that looks peculiar is that in the code above, the first index
of the 2D arrays seems to be a row number, and each 2D array is treated
as if it has AreaRow/res sub-arrays;
while in the code below the first index seems to be a column number, and
each 2D array is treated as if it has AreaCol sub-arrays.

> for (i = 0; i < AreaCol; 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);
> }
> free(work.lightcon2D);
> free(work.bias1002D);
> free(work.bias752D);
> free(work.bias502D);
> free(work.bias252D);
> free(work.latitude2D);
> free(work.longitude2D);
> free(work.valconcen2D);

Can you provide more information (at least the declarations of *all*
the variables and types used in your code fragment)?
--

SDF Public Access UNIX System - http://sdf.lonestar.org

Ike Naar, Jun 9, 2010
12. ### John BodeGuest

On Jun 9, 2:01 pm, Ben Bacarisse <> wrote:
> John Bode <> writes:
> > On Jun 9, 1:15 pm, barry <> wrote:

> <snip>
> >> ii=0;
> >> for (i = 0; i < 300; i++) {
> >>     for (j = 0; j < 250; j++) {
> >>          oneD1[ii] = TwoD1[j];
> >>          oneD2[ii] = TwoD2[j];
> >>          oneD3[ii] = TwoD3[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.
>

Dammit, I should just stop posting. That's like the fifth or sixth
stupid error I've made today. I don't know why I'm having so much
trouble doing simple math, but it's infesting everything I'm doing. I
didn't drink *that* heavily last night.

[snip]

> >  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.

>
>

Well, I got at least that much right anyway...

John Bode, Jun 9, 2010
13. ### Jens Thoms ToerringGuest

Ike Naar <> wrote:
> In article <huoq2q\$bva\$>, barry <> wrote:
> > 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);

> What is the element type of those arrays? If it is something other
> than float* (especially if it is something that has a sizeof larger
> than sizeof(float*), the allocated arrays will be the wrong size.
> Looking at the names, I'd guess that the element types are not float*,
> perhaps they are float or double?

And you also should consider that

sizeof(float*)*AreaRow/res*AreaCol/res

isn't necessarily the same as

sizeof(float*) * (AreaRow/res) * (AreaCol/res)

which I guess is the intended size (modulo the question if
you really want an arrary of float pointers or an array of
floats or perhaps even of something else).

Regards, Jens
--
\ Jens Thoms Toerring ___
\__________________________ http://toerring.de

Jens Thoms Toerring, Jun 9, 2010
14. ### SeebsGuest

On 2010-06-09, barry <> 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:

> MALLOC(ret.lightcon, sizeof(float*)*AreaRow/res*AreaCol/res);

A puzzle for you.

(10/3) * (11/3) == 10/3*11/3

yes/no?

> MALLOC(ret.valconcen, sizeof(int*)*AreaRow/res*AreaCol/res);

This is probably wrong.

> ret.lightcon[ii] = work.lightcon2D[j];

Is this actually an "int *"? Because if this were an array of ints,
it could be that (int *) and (int) are different sizes, which could
result in problems.

Anyway, as suggested: Start trimming this down. Make a minimal program
that populates the various values with likely defaults, and still crashes,
and keep stripping functionality away until you have a simple program
which fails to do nothing, but would do nothing if you could find the bug.

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

Seebs, Jun 9, 2010
15. ### Francois GrieuGuest

On 09/06/2010 20:22, Seebs wrtoe:
> #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[j];
> oneD2[ii] = twoD2[j];
> oneD3[ii] = twoD3[j];
> ++ii;
> }
> }
> printf("%d\n", ii);
> return 0;
> }
>
> This works exactly as expected, and prints "75000".

Nitpick: assuming a hosted implementation with INT_MAX>=75000.

Francois Grieu

Francois Grieu, Jun 10, 2010
16. ### Richard BosGuest

John Bode <> wrote:

> On Jun 9, 1:15=A0pm, barry <> wrote:

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

> 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).

Then he wouldn't get something larger than 300*250 when he prints it
out. (Also, you mean a 17-bit signed int. Or 32767.)

Richard

Richard Bos, Jun 10, 2010