2
int     *rrange(int start, int end);

I was asked, to write a function and allocate with malloc an array of integers, fill it with consecutive values that begin at end and end at start (Including start and end !), then return a pointer to the first value of the array. The functions works with small numbers, when tested with big numbers I get seg fault. And I think I got seg fault due to reaching the limit of int. How can I solve this issue?.

/*- With (1, 3) you will return an array containing 3, 2 and 1
- With (-1, 2) you will return an array containing 2, 1, 0 and -1.
- With (0, 0) you will return an array containing 0.
- With (0, -3) you will return an array containing -3, -2, -1 and 0.*/

#include <stdlib.h>

int *rrange(int start, int end)
{
    int *range;
    int i;

    i = 0;
    range = (int *)malloc(sizeof(int *));
    if (end <= start)
    {
        while (end <= start)
            range[i++] = end++;
    }
    else
    {
        while (end >= start)
            range[i++] = end--;
    }
    return (range);
}

= Test 1 ===================================================
$> ./pw53y11cbachacu14eue5cab 
$> diff -U 3 user_output_test1 test1.output | cat -e

Diff OK :D
= Test 2 ===================================================
$> ./o1jrm4t3vqengizvj1tlwab4 "21" "2313" "12"
$> diff -U 3 user_output_test2 test2.output | cat -e

Diff OK :D
= Test 3 ===================================================
$> ./usl3i1tc1xv9tr1gs9n5x5vr "2147483647" "2147483640" "7"
$> diff -U 3 user_output_test3 test3.output | cat -e
--- user_output_test3   2016-06-08 16:26:16.000000000 +0200$
+++ test3.output    2016-06-08 16:26:16.000000000 +0200$
@@ -1,8 +1,8 @@$
-0$
-0$
-0$
-0$
-0$
-0$
-0$
+2147483640$
+2147483641$
+2147483642$
+2147483643$
+2147483644$
+2147483645$
+2147483646$
$

Diff KO :(
Grade: 0

= Final grade: 0         ===============================================================
Junius L
  • 15,881
  • 6
  • 52
  • 96

5 Answers5

4

Your problem comes from the malloc:

Instead of

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

You should write:

range = malloc(nb_of_integer_in_range * sizeof (int));

Up to you to compute nb_of_integer_in_range, something like:

int nb_of_integer_in_range;
if (end > start)
{
    nb_of_integer_in_range = end - start + 1;
}
else ...
Mathieu
  • 8,840
  • 7
  • 32
  • 45
  • 1
    [reason for not casting the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) – phuclv Jun 09 '16 at 06:33
  • You can always use the size of the defererenced pointer itself in you `malloc` call to prevent any mixup, e.g. `range = malloc(nb_of_integer_in_range * sizeof *range);` – David C. Rankin Jun 09 '16 at 06:48
  • @purplepsycho Thanks, but I'm still getting `seg fault` http://code.geeksforgeeks.org/ak4BSE – Junius L Jun 09 '16 at 06:57
  • @TenTenPeter aren't you missing a `+1` in linked code? – Mathieu Jun 09 '16 at 07:04
  • @purplepsycho no, I'm still getting `seg fault` even after adding `+1` – Junius L Jun 09 '16 at 07:12
  • @TenTenPeter The problem you have in linked code is linked to `end<=start` comparison: As `start` is equal to the maximum integer representable in `int32_t`, the test `end <= start` will always be **true**: when `end` is equal to `start`, you increase `end` which becomes `0`... – Mathieu Jun 09 '16 at 08:21
  • @purplepsycho I was telling myself that the `<=` were going to cause problems. – Junius L Jun 09 '16 at 08:24
1

segfault occurs due to wrong memory allocation

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

this would allocate a memory of the size of an integer pointer. but to allocate memory for numbers you need to create required amount of memory for each number this way :

range = malloc(no_of_elements*sizeof(int));

NOTE: use n as global variable so that you can know the size of array even in function() where you print

and I've modified your function

int *rrange(int start, int end)
{
    int *range;
    int i;

    if(start>end)
        return rrange(end,start); //so that start is always <end

    n=end-start+1; // n globally declared 

    range=malloc(n*sizeof(int)); //casting is not required
    if(range==NULL)
    {// check if memory was successfully allocated if not exit
        printf("fail");
            exit(1);
    }
    for(i=0;i<n;i++,start++)
    {
        range[i]=start;
    }

    return range;
}

you can now write your main as:

int main()
{
    int x,y,*r,i;
    scanf("%d%d",&x,&y);
    r=rrange(x,y);
    for(i=0;i<n;i++)
    {
        printf("%d,",r[i]);
    }
    return 0;
}
Cherubim
  • 5,287
  • 3
  • 20
  • 37
0

I'm surprised this worked with any range larger than 2. You're allocating 8B of storage (assuming you're running a 64b machine), or whatever is pointer size in general. What you want instead is a malloc expression that depends on the difference between start and end (giving you the array size) and sizeof(int) (giving you the array element size).

Unrelated note: make sure to free the storage when you stop using the array, otherwise your program will leak like a sieve.

starturtle
  • 699
  • 8
  • 25
0

The argument to malloc should be number_of_integers * sizeof(int). The value of number_of_integers can for example be calculated as abs(start-end)+1, but I'd instead do the malloc inside the if-statement where the relation between start and end is known.

MicroVirus
  • 5,324
  • 2
  • 28
  • 53
  • using the calculation: `abs(start-end)+1` and the values: start=-1, end=3) the result would be abs(2)+1 yields 3 however that value yield should be 5. Suggest: `abs( end - start ) + 1` – user3629249 Jun 09 '16 at 17:30
0

the following code:

  1. cleanly compiles
  2. performs the desired function
  3. properly handles error conditions
  4. might not work if start is greater than end however, that was not part of the criteria

and now the code:

/*- With (1, 3) you will return an array containing 3, 2 and 1
- With (-1, 2) you will return an array containing 2, 1, 0 and -1.
- With (0, 0) you will return an array containing 0.
- With (0, -3) you will return an array containing -3, -2, -1 and 0.*/

#include <stdlib.h> // exit(), EXIT_FAILURE
#include <stdio.h>  // perror()
#include <math.h>   // abs()

int *rrange(int start, int end)
{
    int *range = NULL;

    if( NULL == (range = malloc( sizeof(int) *((size_t)abs(end - start) +1) ) ) )
    { // then malloc failed
        perror( "malloc failed" );
        exit( EXIT_FAILURE );
    }

    for( int index = 0; index < abs(end-start)+1; index++ )
    {
        range[ index ] = start+index;
    }

    return (range);
} // end function: rrange
user3629249
  • 16,402
  • 1
  • 16
  • 17
  • Thanks, but I'm not allowed to use `` , `` and `for` loops. the only allowed function is `malloc`. – Junius L Jun 09 '16 at 17:31
  • @TenTenPeter. To generate all those numbers, your going to needs some kind of loop. And `malloc()` is a function call while `for()` is a keyword in the language. Rather than using `abs()`, you could make the calculation into a stack variable, then test, using `if()` to check if the result is negative, then if negative, multiply by -1. I would expect that your instructor would never suggest using a `goto` statement, as such a statement (almost) always a bad idea and a sign of poor algorithm development. Will your instructor let you use a `while()` statement? – user3629249 Jun 10 '16 at 06:39
  • @TenTenPeter. it is very poor programming practice to not check for error conditions returned from system functions. and announcing such an error requires I/O which requires `stdio.h` – user3629249 Jun 10 '16 at 06:42