bad output in printing even/odd numbers in array - c

Question

I have an array of numbers .. what is needed : to create 2 new arrays by malloc one for even numbers and one for odd numbers and we need to print them in main with the size of each array too

Array is 1,2,5,6,8,4,5,10,65,69,98,76,46,49,67

Even Array must be: 2,6,8,4,10,98,76,46

Odd Array must be: 1,5,5,65,69,49,67

The function must be written like this (this is how the teacher wants)

void evenodd(int a[],int** even,int* evensize,int** odd,int* oddsize)

my code:

#include <stdio.h>
#include <stdlib.h>
#define N 15
void evenodd(int a[],int** even,int* evensize,int** odd,int* oddsize)
{
    int i,j,cnte,cnto,e,o;
    cnte=cnto=0;
    for(i=0;i<N;i++)
    if(a[i]%2==0) cnte++;
    else cnto++;

    even=(int**)malloc(sizeof(int)*cnte);
    *evensize=cnte;
    odd=(int**)malloc(sizeof(int)*cnto);
    *oddsize=cnto;

    for(i=0,o=0,e=0;i<N;i++)
        if(a[i]%2==0)
        even[e++]=a[i];
        else odd[o++]=a[i];
  return;
}

int main()
{
    int i;
    int a[N]={1,2,5,6,8,4,5,10,65,69,98,76,46,49,67};
int *even,evensize,*odd,oddsize;
evenodd(a,&even,&evensize,&odd,&oddsize);
printf("Even Numbers Array: ");
for(i=0;i<evensize;i++)
printf("%d ",even[i]);
printf("\nSize of even array is: %d",evensize);
    printf("\n\n");
printf("Even Numbers Array: ");
for(i=0;i<oddsize;i++)
    printf("%d ",odd[i]);
printf("\nSize of even array is: %d",oddsize);
}

i think the error is in Malloc .. but i dont know why ..

@PhilipConrad Well, where you have been since this spring? :) The `homework` tag is deprecated. — , Jul 20 '13 at 18:37
@H2CO3 Whoops! My bad. I haven't been on SO recently, so I probably missed that announcement. — Philip Conrad, Jul 20 '13 at 18:40
@PhilipConrad Yeah, it wasn't too long time ago. The SO staff argued that it's not useful, so it should not be used anymore. — , Jul 20 '13 at 18:40

score 4 · Answer 1 · answered Jul 20 '13 at 18:24

4

Update allocation as

*even = malloc(sizeof(int)*cnte);
*odd = malloc(sizeof(int)*cnto);

and use as

for(i=0,o=0,e=0;i<N;i++)
    if(a[i]%2==0)
      (*even)[e++]=a[i];
    else 
      (*odd)[o++]=a[i];

answered Jul 20 '13 at 18:24

Rohan

52,392
12
90
87

+1, would be useful to insert a link to the standard "don't cast malloc" answer. – Jul 20 '13 at 18:27

score 3 · Answer 2 · edited May 23 '17 at 12:21

Your malloc calls are:

even=(int**)malloc(sizeof(int)*cnte);
...
odd=(int**)malloc(sizeof(int)*cnto);

Your first clue that this is wrong should be that even and odd are output parameters to your evenodd function, but here you're modifying local variables. C passes arguments by value; in order to modify objects passed by the caller, you must add a level indirection (a pointer). (See my answer to C Programming: malloc() inside another function for an explanation about this.)

Therefore the malloc calls should be:

*even=malloc(sizeof(int)*cnte);
...
*odd=malloc(sizeof(int)*cnto);

Note that in C, it is unnecessary to explicitly cast the result of malloc (and doing so can hide errors). It's also generally recommended to use p = malloc(sizeof *p) instead of p = malloc(sizeof (type)) so that if the type of p changes, you don't silently allocate an incorrect buffer size:

*even=malloc(sizeof **even * cnte);
...
*odd=malloc(sizeof **odd * cnto);

jxh · Accepted Answer · 2013-07-20T18:58:53.383

Since C function calls use pass by value, to pass back a value through an out parameter, you need a pointer to the variable that will hold the value being passed back. But, within that function, you need to dereference the pointer to change the value stored in the variable so that the caller sees the value being passed back. (You get this right for your counter function parameters, evensize and oddsize.)

To take an example, your even array is being assigned like this:

    even=(int**)malloc(sizeof(int)*cnte);

When you dynamically allocate an item (or in this case an array of items) of type FOO, malloc() logically returns a FOO *. You should not cast the return value of malloc(), but your cast is wrong anyway, it should have been:

    even=(int*)malloc(sizeof(int)*cnte);

However, even is an int **, which is a pointer to the int *even variable in main(). To let main() see the result, you have to dereference the pointer passed to your function. As mentioned previously, you should not cast the result of malloc(), as it can mask an error if its prototype is missing, which can lead to undefined behavior in the worst case.

    *even=malloc(sizeof(int)*cnte);

In C, the declaration of the variable mimics use. So int **even means that **even is an int. So, to assign an int value properly, even has to be dereferenced twice. The first time to get to the array allocated by malloc(), and a second time to reach the desired index position. So, your assignment statement:

        even[e++]=a[i];

is wrong because it is assigning an int value to an int * type since even is only dereferenced once. Since even is a pointer to an int *, use:

        (*even)[e++] = a[i];

Similarly, make the same corrections for the odd function parameter.

bad output in printing even/odd numbers in array - c

3 Answers3