How to I operate on int arrays declared using malloc?

Question

I have this piece of code:

// Returns the fibonacci range until the specified limit
int fibo(int** output, int limit)
{
    // Must be 1 or more
    if(limit < 1) return 0;

    int* range = (int*)malloc(sizeof(int) * limit);
    assert(range);

    int i;

    // Calculate the range
    for(i = 0; i < limit; i++)
    {
        int value;

        if(i == 0)  value = 0;
        if(i == 1)  value = 1;
        else        value = range[i - 2] + range[i - 1];

        range[i] = value;
    }

    *output = range;

    return 1;
}

Running it with limit 15 outputs

65, 1, 66, 67, 133, 200, 333, 533, 866, 1399, 2265, 3664, 5929, 9593, 15522

which is not right at all. I suspect it's because I'm writing stuff like range[i - 2] when that's not what I should be doing. I tried using the size of int as the hop between each value and got segmentation errors. Am I using [] correctly? Can anyone think of any other reason why my output is weird?

Here's all the code for the program

`range[i-2]` is fine so long as `i > 2`. Have you tried stepping through your code in a debugger? — Oliver Charlesworth, Sep 06 '11 at 00:37
It's just a simple bug. Consider carefully what your code does for `i == 0`... (Hint: You wanted "else if" for the second arm) — Nemo, Sep 06 '11 at 00:38
Tangential note: The canonical idiom for `malloc` is `int *range = malloc(sizeof(*range) * limit)`. Don't cast, and don't refer to hard-coded types in `sizeof`. — Oliver Charlesworth, Sep 06 '11 at 00:39
@bmargulies: See this: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc. — Oliver Charlesworth, Sep 06 '11 at 00:40
Hats of to Nemo! I didn't notice it. Please make it an answer. Also thanks Oli Charlesworth for correcting bad code. However I'm not sure I understand why I shouldn't use sizeof(int). My output should in all cases be an array of ints, after all. Could you elaborate? — Hubro, Sep 06 '11 at 00:43
It's a lot saner, by the way, to have the caller provide the array to this function than to deal with double indirection. ** in a C program is a bad code smell. — Russell Borogove, Sep 06 '11 at 00:44
@Codemonkey: Consider what happens if later you change `range` to a different data type, and are in a hurry. — Oliver Charlesworth, Sep 06 '11 at 00:44

score 1 · Accepted Answer · answered Sep 06 '11 at 00:40

1

Change

if(i == 1)  value = 1;

to

else if(i == 1)  value = 1;

EDIT: Just realized this was already answered in the comments.

answered Sep 06 '11 at 00:40

Mysticial

464,885
45
335
332

score 0 · Answer 2 · answered Sep 06 '11 at 00:43

0

The issue is with your ifs You have two if statements

if(i == 0)  value = 0;

and

if(i == 1)  value = 1;
  else        value = range[i - 2] + range[i - 1];

If i is 0 then the second if evaluates to range[-2] + range[-1] so undefined data from memory

You need to be using else so that it is just one if statement (also as a point of style always use {} to make things clearer)

if(i == 0) {  
   value = 0;
} else if(i == 1) {
   value = 1;
} else {
   value = range[i - 2] + range[i - 1];
}

In this example probably even better to set range[0] and [1] before the loop and start the loop at 2 so no need for the if.

answered Sep 06 '11 at 00:43

mmmmmm

32,227
27
88
117

1

+1 for the mention of starting the loop at 2, -1 for the style recommendation, whose clearness is clearly subjective. – Christian Rau Sep 06 '11 at 00:49
I considered starting the loop at 2 before I even started writing, but how then would I be able to run the function with `limit=1`? – Hubro Sep 06 '11 at 00:51

score 0 · Answer 3 · answered Sep 06 '11 at 00:43

0

You're missing an else between the if (i==0) and if (i == 1), so the first time through both the 0 case and the 2+ case get run.

answered Sep 06 '11 at 00:43

Russell Borogove

18,516
4
43
50

How to I operate on int arrays declared using malloc?

3 Answers3