0

Hey I'm trying to switch the two parameters around only when parameter 1 is greater than parameter 2.

What I've got so far is this:

#include <stdio.h>

void ascending2(int*, int*);

int main(void){
    int ptr1 = 20;
    int ptr2 = 10;
    printf("ptr1 = %d, ptr2 = %d", ascending2(&ptr1, &ptr2));
    return 0;
}

void ascending2(int *ptr1, int* ptr2){
    int *value; 
    if( *ptr1 >= *ptr2 ){
         *value = *ptr1;
         *ptr1 = *ptr2;
         *ptr2 = *value;}
}

Can anyone suggest where I may be going wrong, thanks.

2
  • I'm not sure what the problem really is. Commented Aug 27, 2014 at 4:48
  • Not a bug, but the variable names ptr1 and ptr2 in main() are really bad. Commented Aug 27, 2014 at 7:47

5 Answers 5

4

It's because in the ascending2 function you declare value as a pointer, but you don't actually make it point anywhere. So when you dereference it you dereference an uninitialized pointer which leads to undefined behavior.

There are two obvious solutions: Either you make value point somewhere valid, or even better you declare it as a non-pointer variable.


There also the problem with your printf call, as ascending2 is declared to return void, in other words it doesn't return anything at all, and yet you print two values. This will also lead to undefined behavior.

The solution here is to call the function separately, and then call printf with the ptr1 and ptr2 variables (which by the way are not very well named, as they are not actually pointers).

Sign up to request clarification or add additional context in comments.

Comments

2
printf("ptr1 = %d, ptr2 = %d", ascending2(&ptr1, &ptr2));

here ascending2(..) not returning anything. Require to change printf. like

 ascending2(&ptr1, &ptr2);
 printf("ptr1 = %d, ptr2 = %d",ptr1,ptr2);

And your function should be

void ascending2(int *ptr1, int* ptr2){
    int value;                     // change *value to value
    if( *ptr1 >= *ptr2 ){
         value = *ptr1;            // change *value to value
         *ptr1 = *ptr2;
         *ptr2 = value;}           // change *value to value
}

Comments

1

The value you declare is a pointer. Change it to int value.

Comments

1

Here, the mistakes are corrected.

#include <stdio.h>

void ascending2(int*, int*);

int main(void){
    int ptr1 = 20;
    int ptr2 = 10;
    ascending2(&ptr1, &ptr2); //Fix1

    printf("ptr1 = %d, ptr2 = %d", ptr1, ptr2);
    return 0;
}

void ascending2(int *ptr1, int* ptr2){
    int value=0; //Fix2
    if( *ptr1 >= *ptr2 ){
         value = *ptr1;
         *ptr1 = *ptr2;
         *ptr2 = value;}
}

Fix1: Argument should not be passed to printf statement as you did.The function ascending2 also not returning any values. So call ascending2 API 1st and then print the values of ptr1, ptr2. To know more about printf please refer the below link. http://www.cplusplus.com/reference/cstdio/printf/

Fix2:

Inside ascending2 function no need to use value as a pointer variable. It can be just integer. If you use value as pointer, then proper memory should be provided.

Comments

0

Just to add some suggestions, there are already good answers: the swap function can be improved:

void ascending2(int *ptr1, int* ptr2)
{
  if( *ptr1 > *ptr2 )
  {
    const int tmp = *ptr1;
    *ptr1 = *ptr2;
    *ptr2 = tmp;
  }
}

The improvements:

  • Use > for the comparison, there's no need to do anything if the two values are equal so comparing using >= makes no sense.
  • Moved the temporary variable into the scope where it's used.
  • Renamed it to tmp to make it a bit clearer what it does. "Value" is really generic.
  • Made it const since it doesn't change after being assigned.
  • Initialize directly with the desired value.

2 Comments

Thanks, what does 'tmp' stand for?
@IamTrent It's a commonly used contraction of "temporary".

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.