1

I was given the assignment of writing a C code which contains a function, which when passed through an array of length n returns a pointer to the array’s largest element. In order to make it easy for us, the length n was set to 10.

I wrote the code below and compiled it and it didn't show any error. Then, I ran this code and it asked for the 10 inputs for the array, but instead of returning the pointer with the largest element, it read segmentation fault 11.

I am new to pointers, and so could not figure out what exactly went wrong. Any help will be greatly appreciated. 

#include<stdio.h> #define N 10 int *find_largest(int a[], int n) { int i; int *max; *max = a[0]; for(i = 1; i < n; i++) { if(a[i] > *max) *max = a[i]; } return max; // this function returns the pointer with the max. value } int main(void) { int b[N], i; int *p; printf("Enter %d numbers: ", N); for(i = 0; i < N; i++) scanf("%d", &b[i]); //scans all the inputs p = find_largest(b, N); //p gets assigned the value of the max valued pointer printf("largest element: %d", *p); return 0; }

Edit- After looking at the wonderful advices given, I modified my code a bit. I changed *max=a[0] to max= &a[0], and *max=a[i] to max=&a[i]. Now, the code runs without any error, but instead of returning the pointer with the largest value, it returns the largest input instead.
screenshot of what is happening

Improve this question
10
  • 2
    *max = a[0]; -> max = &a[0]; and similar elsewhere. Commented Apr 30, 2018 at 6:01
  • 1
    @Mulliganaceous Signal number, not errno. Commented Apr 30, 2018 at 6:03
  • 1
    @DavidC.Rankin The assignment says the function needs to return a pointer. Commented Apr 30, 2018 at 6:07
  • 1
    A function can always return its own type -- so I guess leaving it as int * would be just what the prof ordered here :) Commented Apr 30, 2018 at 6:16
  • 1
    Compile with all warnings and debug info: gcc -Wall -Wextra -g with GCC. Then use the gdb debugger to understand the faulty behavior of your program Commented Apr 30, 2018 at 6:40

2 Answers 2

Reset to default
3

In this line, you write to the place the uninitialised ponter is pointing to.

*max = a[0];

This causes undefined behaviour (or "UB", reading up on it is worth the time) and to be strictly avoided; among other reasons because it is a very likely reason for segfaults.

What you probably want to do is to write the address of the first element into the pointer.

max = &a[0];

Later you write the place the pointer is pointing to, updating it to the max value you have found.

*max = a[i];

Guessing from the rest of your code, you probably want to just keep the address of the max element, so that you can return its address as it is in the input array, without changing the values in the input array (which your currently do, and it seems unlikely that you intend to...).

max = &a[i];

Referring to your comment (that the value is returned, not the pointer):
For returning a pointer (not a value), make sure that you

return max; // the pointer, not the value it is pointing to

Which is what you actually do in the currently shown code version (possibly before an edit...).
Do not return *max; that would return the value.

If by "return" you mean "print", then you need to change the

printf("largest element: %d", *p);

Use p instead of *p and change the format specifier accordingly.
You probably want "largest element at address: %p". (Compare http://en.cppreference.com/w/c/io/fprintf )

printf("largest element at address: %p", p);
Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

well, yes I want to keep the address of the max element and return that instead of the actual "value" of the largest element. Right now, the code returns the largest input instead of the pointer to which it's assigned.
printf(largest element: %p", p); does compile and run without a problem, but the output is now neither a number or the pointer, but rather a hexadecimal output largest element: 0x7ffeeb764af4
That is on your environment the representation of a pointer and the way it is supposed to be done. What format of output do you expect?
output should in the form of a[i], as that's how I defined it initially for the element max
Please elaborate that, it is unclear to me. What do you mean by "form of a[i]"? And what do you mean by "how I defined for the element max"? You defined max as a pointer to int. That does not imply a specific form (apart from the standard, implementation specific way as implied by "%p"). a[i] in itself does not imply any format either.
|
2 
 int *max; *max = a[0];

This is UB (undefined behavior) since max is not pointing to valid memory and you try to dereference it.

max = &a[0];

Moreover your logic to find the max is wrong. Why do you reset it every time to 0th element. It should be something like.

for(i = 1; i < n; i++) { if(a[i] > *max) max = &a[i]; }
Improve this answer

1 Comment

I did change my code the way you asked me to, but now instead of returning the pointer with the largest element, it's returning the largest element itself. Does this have to do with changing max=&a[o]?

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.