Using more elements in array than allocated has undefined behavior

Question

I have structure of triangles, user must enter number of triangles that he wants to have and then fill data for each one - length of edges. Then program writes edge with biggest length. First I allocate memory for my triangle array, and then I fill each triangle with data, but my program never saves last entered triangle in my array and I get some weird number as biggest edge. Here is my code

struct triangle
{
    int a;
    int b;
    int c;
};

int getBiggestEdge(int a, int b, int c)
{
    int max = a;
    if(b>max) {max=b;}
    if(c>max) {max=c;}
    return max;
}

int maxEdge(struct triangle niz [], int br)
{
    int i, najduza;
    najduza = getBiggestEdge(niz[0].a, niz[0].b, niz[0].c);

for(i=1; i<=br; i++)
{
    if(getBiggestEdge(niz[i].a, niz[i].b, niz[i].c) > najduza)
    {
        najduza = getBiggestEdge(niz[i].a, niz[i].b, niz[i].c);
    }
}
return najduza;
}

int main()
{
    int i, n, edge;
    struct triangle* niz;

    printf("Insert number of triangles: ");
    scanf("%d", &n);

    niz= (struct triangle*) malloc(n * sizeof(struct triangle));

    if (niz == NULL) 
    { 
        printf("Error!");
        return 0;
    }

    printf("Insert data for every triangle (a b c):\n");

    for(i=0;i<n;i++)
    {
        printf("Triangle: ");
        scanf("%d %d %d", &niz[i].a, &niz[i].b, &niz[i].c);
    }

    edge = maxEdge(niz, n);

    printf("Biggest edge is: %d\n",edge);

    free(niz);
    return 0;
}

from the comment to the Devolus answer, we deduce you are indeed using a C++ compiler. This does not matter too much here (except for casting `malloc` result), but pay attention: C and C++ should be consdered two distinct languages — ShinTakezou, Jan 18 '14 at 12:42

score 2 · Answer 1 · edited May 23 '17 at 11:57

2

You are using the wrong index in maxEdge:

for(i=1; i<=br; i++)

it should be

for(i = 0; i < br; i++)

In the main loop you used it correct, but in the function you are reading beyond the array boundary and get undefined data.

Unrelated but worth mentioning, you shouldn't cast the pointer returned by malloc:

Do I cast the result of malloc?

edited May 23 '17 at 11:57

Community

1
1

answered Jan 18 '14 at 12:27

Devolus

21,661
13
66
113

This is not problem. I declare my max edge as one edge from 0-index element from my triangles array, so I don't need to compare 0-index element with 0-index element again, and because of that I started my for loop at 1. And second, if I dont cast pointer returned by malloc I get error: 3 IntelliSense: a value of type "void *" cannot be assigned to an entity of type "triangle *" – Mahir Duraković Jan 18 '14 at 12:34
then you are compiling with C++ compiler and not with a C compiler: in C, you don't need the cast. If `i=1` is correct, it is not `i <= br` anyway, if br is the number of elements. – ShinTakezou Jan 18 '14 at 12:40
If you get an error on malloc you are using a C++ compiler and then you need the cast. Your wrong value is deifnitely what I showed you as the problem though. – Devolus Jan 18 '14 at 12:40
1

If you don't believe it to tbe the problem, then test it with the trivial case of `n = 1` then you can easily debug it. a[0] will be the data, but you are accssing a[1], so I hope you see th eproblem. – Devolus Jan 18 '14 at 12:44

Vikas Sangle · Accepted Answer · 2014-01-18T13:09:44.220

2

You are allocating space for n array elements with malloc(n * sizeof(struct triangle)) and taking input for n elements with:

for(i=0;i<n;i++)

but accessing n+1 elements with:

for(i=1; i<=br; i++)

in maxEdge. Here br has the value of n, so the code accesses n+1 elements (indexed from 0 to n), which has undefined behavior.

edited Jan 18 '14 at 13:09

answered Jan 18 '14 at 12:36

Vikas Sangle

632
5
19

Using more elements in array than allocated has undefined behavior

2 Answers2