C programming - array / value of a pointer comparisson

Question

I don't have much C experience or knowledge and I got stuck with some homework problem,

I try to build a program to find the min and max of an array using (3/2)*n comparisons, I went through a lot of Q&A here and it helped a lot, now everything with it is ok other than a weird problem I can't seem to solve.

When I try to compare if(a[0]>a[1]) or if(*(a)>*(a+1)) everything is ok, when I try to use else after those phrases or try to if(a[1]>a[0]) or if(*(a+1)>*(a)) then the program dies.

#include <stdio.h>
#include <stdlib.h>

void maximum(int *a, int n, int *max, int *min);

    int main()
    {
        int i;
        int number;
        int *max;
        int *min;
        int *a;
        puts("hello, pls enter the number of numbers");
        scanf("%d",&number);
        a = (int*)calloc(number,sizeof(int));
        puts("enter the numbers");
        for(i=0;i<number;i++)
        {
            printf("%d",i);
            puts("before");
            scanf("%d",&a[i]);
            puts("after");
        }

       puts("ok");
        maximum(a,number,max,min);
        printf("min is %d, max is %d",*min,*max);
        return(0);


    }

    void maximum(int *a, int n, int *max, int *min)
    {
        int i;
        int temp;
        int tempmax;
        int tempmin;

        if(n==0)
            {
                puts("come on, be serius");
                return(0);
            }
        if(n==1)
            {
                puts("the number you entered is the min and the max but where is the challenge?");
                *min=*max=a[0];
            }
        puts("check3");

       **if(*(a+1)>*(a))
        {
            *max=a[1];
            *min=a[0];
        }**



        /*if(a[0]>a[1])
        {
            *max=a[0];
            *min=a[1];
        }else
        {
            *max=a[1];
            *min=a[0];
        }

        for(i==2;i<(n-2);i=i+2)
        {
            if(a[i]>a[i+1])
            {
                tempmax=a[i];
                tempmin=a[i+1];
            }else
            {
                tempmin=a[i];
                tempmax=a[i+1];
            }
            if(tempmax>*max)
                *max=tempmax;
            if(tempmin<*min)
                *min=tempmin;
        }
    puts("check5");
        if((n%2)==1)
        {
            if(a[n-1]<*min)
                *min=a[n-1];
            if(a[n-1]>*max)
                *max=a[n-1];
        }/*else
        {
            if(a[i]>a[i+1])
            {
                tempmax=a[i];
                tempmin=a[i+1];
            }else
            {
                tempmin=a[i];
                tempmax=a[i+1];
            }
            if(tempmax>*max)
                *max=tempmax;
            if(tempmin<*min)
                *min=tempmin;
        }
        */


    }

Also, the `min` and `max` variables in `main` don't actually point anywhere. You should declare them as `int` instead of `int *`, and call `maximum(a,number,&max,&min)`, and not dereference them in the following `printf`. — pat, Dec 31 '14 at 09:26
Note that the cast of `(int*)` is not needed in `a = (int*)calloc(number,sizeof(int));`. Curious: why was it coded that way? — chux - Reinstate Monica, Dec 31 '14 at 09:40

score 1 · Answer 1 · edited May 23 '17 at 10:33

1

In your code [apparently commented], inside void maximum() function, int i is not initialized and afterwards, by mistake, instead of initializing =, you're comapring == and using value of i. [Read-before-write scenario]. With uninitialized i, a[i] can be very well produce undefined behaviour.

for(i==2;i<(n-2);i=i+2)

should be

for(i=2;i<(n-2);i=i+2)
     ^
     |

In the same function, you have used int *max and int *min, dereferencing them without any NULL check. From the main(), you're supplying uninitialized pointers. This is also undefined behaviour and can certainly cause segmentation fault.

Also, you never checked for the success of scanf().

Then, never use return(0);from a function of which the return type is void. (thanks to @pat for below comment) .

Next, please do not cast the return value of malloc()/ calloc().

edited May 23 '17 at 10:33

Community

1
1

answered Dec 31 '14 at 09:17

Sourav Ghosh

133,132
16
183
261

Or `return(0)` from a `void` function. – pat Dec 31 '14 at 09:23
hey, thanks. what do you mean by "Then, never user return(0);, as mentioned by @pat in below comment, from a function of which the return type is void." – thebeancounter Jan 10 '15 at 11:11
@captainshai so sorry, typo. read it as `never use return(0)...` or have a look at my updated answer. :-) – Sourav Ghosh Jan 10 '15 at 11:14

Gopi · Answer 2 · 2014-12-31T10:04:39.813

1

Check the code below:

#include <stdio.h>
#include <stdlib.h>

void maximum(int *a, int n, int *max, int *min);

    int main()
    {
        int i;
        int number;
        int max;
        int min;
        int *a;

        scanf("%d",&number);
        a = malloc(sizeof(int) * number);

        for(i=0;i<number;i++)
        {
            scanf("%d",&a[i]);
        }


        maximum(a,number,&max,&min);
        printf("min is %d, max is %d",min,max);
        return(0);


    }

   void maximum(int *a, int n, int *max, int *min)
{
    int i,ma=a[0],mi = a[0],t1,t2;
    for(i=n%2;i<(n-1);i= i+2)
    {
            if(a[i] > a[i+1])
            {
              t1 = a[i];
              t2 = a[i+1];
            }
            else
            {
                t1 = a[i+1];
                t2 = a[i];
            }

            if(t1 > ma)
            ma = t1;

            if(t2<mi)
            mi = t2;
    }
    *max = ma;
    *min = mi;
}

edited Dec 31 '14 at 10:04

answered Dec 31 '14 at 09:27

Gopi

19,784
4
24
36

This solution uses `2n` compares, as opposed to the `3n/2` compares that the OP was trying for. – pat Dec 31 '14 at 09:29
@pat Cool will fix it – Gopi Dec 31 '14 at 09:36
You may find `a = malloc(number * sizeof *a);` even a better style when calling `malloc()` and friends. IMO, easier to code & maintain and less prone to error. – chux - Reinstate Monica Dec 31 '14 at 09:44
This and likely OP's code has trouble if `n` is even. Consider `n==2`, `a[1]` is never accessed. An easy fix would be `for(i=(n%2);i<(n-1);i= i+2)` – chux - Reinstate Monica Dec 31 '14 at 09:59
hey, thanks for all the answers, i will try again and hope it will work, by the way, i am new here, pls advise, does op stand for original program or as trash talk towards me? – thebeancounter Jan 01 '15 at 12:09
@captainshai `Original poster` :) Be Happy nothing against you :) – Gopi Jan 01 '15 at 12:11
hey chuks, thanks, but if i use malloc with (number * sizeof *a), wouldnt it allways give me for bytes as a pointer's space? will it do for a pointer to a long variable as an instance? – thebeancounter Jan 10 '15 at 11:13
@captainshai Using malloc() you will get number of bytes needed to write to your array so you can do as shown in the code – Gopi Jan 10 '15 at 11:17

tapananand · Answer 3 · 2014-12-31T09:21:22.977

0

In the for loop you have used == instead of = and i is not initialiazed before. So i has garbage value causing seg fault.

edited Dec 31 '14 at 09:21

answered Dec 31 '14 at 09:15

tapananand

4,392
2
19
35

score 0 · Answer 4 · answered Dec 31 '14 at 09:30

Your program dies because you don't define *max and *min values when your comparation fails. You need to add something like:

else
{
        *min=a[1];
        *mmax=a[0];
}

This won't solve your homework completely. If you want to find a maximum or minimum from a array, you must do some comparing inside a loop.

score 0 · Answer 5 · answered Jan 10 '15 at 11:18

thanks for all the help. these were the issues with the program, and for the (3n/2) part, the solution was to take each couple of numbers, and to find a local min and max as following:

if(a>b)
  {
    max = a;
    min = b;
  } else 
  {
    max=b;
    min=a;
  }

and then compare every max with the following max and min to the following min as in

  if(max1>max2) 
    totalmax=max1;
  else 
    totalmax=max2;

C programming - array / value of a pointer comparisson

5 Answers5