views:

1425

answers:

6

This is going to sound like a silly question, but I'm still learning C, so please bear with me. :)

I'm working on chapter 6 of K&R (structs), and thus far through the book have seen great success. I decided to work with structs pretty heavily, and therefore did a lot of work early in the chapter with the point and rect examples. One of the things I wanted to try was changing the canonrect function (2nd Edition, p 131) work via pointers, and hence return void.

I have this working, but ran into a hiccup I was hoping you guys could help me out with. I wanted canonRect to create a temporary rectangle object, perform its changes, then reassign the pointer it's passed to the temporary rectangle, thus simplifying the code.

However, if I do that, the rect doesn't change. Instead, I find myself manually repopulating the fields of the rect I'm passed in, which does work.

The code follows:

#include <stdio.h>

#define min(a, b) ((a) < (b) ? (a) : (b))
#define max(a, b) ((a) > (b) ? (a) : (b))

struct point {
    int x;
    int y;
};

struct rect {
    struct point lowerLeft;
    struct point upperRight;
};

// canonicalize coordinates of rectangle
void canonRect(struct rect *r);

int main(void) {
    struct point p1, p2;
    struct rect r;

    p1.x = 10;
    p1.y = 10;
    p2.x = 20;
    p2.y = 40;
    r.lowerLeft = p2; // note that I'm inverting my points intentionally
    r.upperRight = p1;

    printf("Rectangle, lower left: %d, %d; upper right: %d %d\n\n", 
        r.lowerLeft.x, r.lowerLeft.y, r.upperRight.x, r.upperRight.y);

    // can't pass a pointer, only a reference. 
    // (Passing pointers results in illegal indirection compile time errors)
    canonRect(&r); 
    printf("Rectangle, lower left: %d, %d; upper right: %d %d\n\n", 
        r.lowerLeft.x, r.lowerLeft.y, r.upperRight.x, r.upperRight.y);    
}

void canonRect(struct rect *r) {
    struct rect temp;
    temp.lowerLeft.x = min(r->lowerLeft.x, r->upperRight.x);
    temp.lowerLeft.y = min(r->lowerLeft.y, r->upperRight.y);
    temp.upperRight.x = max(r->lowerLeft.x, r->upperRight.x);
    temp.upperRight.y = max(r->lowerLeft.y, r->upperRight.y);

    r = &temp; // doesn't work; my passed-in rect remains the same

    // I wind up doing the following instead, to reassign all 
    // the members of my passed-in rect
    //r->lowerLeft = temp.lowerLeft;
    //r->upperRight = temp.upperRight;
}

So here are the questions:

  1. Why does r = &temp; not work? (I think this is because I pass in a reference instead of a pointer; am I correct in thinking that references are not modifiable but pointers are?)
  2. Why might I get an illegal indirection compile-time error if I try to pass in a pointer to canonRect? (IE, if I had canonRect(*r); in main().)

I suspect I already know the answer to #1, but #2 perplexes me -- I thought it was legal to pass pointers around.

Anyway ... please forgive the C newb.

+5  A: 

I think what you want to do is this:

void canonRect(struct rect *r) {
    struct rect temp;
    temp.lowerLeft.x = min(r->lowerLeft.x, r->upperRight.x);
    temp.lowerLeft.y = min(r->lowerLeft.y, r->upperRight.y);
    temp.upperRight.x = max(r->lowerLeft.x, r->upperRight.x);
    temp.upperRight.y = max(r->lowerLeft.y, r->upperRight.y);

    *r = temp; 
}

In the above code you are setting *r which is of type rect to temp which is of type rect.

Re 1: If you want to change what r is pointing to you need to use a pointer to a pointer. If that's really what you want (see above, it is not really what you want) then you'd have to make sure to point it to something on the heap. If you point it to something not created with 'new' or malloc then it will fall out of scope and you will be pointing to memory that is no longer used for that variable.

Why doesn't your code work with r = &temp?

Because r is of rect* type. That means that r is a variable that holds a memory address who's memory contains a rect. If you change what r is pointing to, that's fine but that doesn't change the passed in variable.

Re 2: * when not used in a type declaration is the dereference unary operator. This means that it will lookup what is inside the address of the pointer. So by passing *r you are not passing a pointer at all. In face since r is not a pointer, that is invalid syntax.

Brian R. Bondy
Ehhh ... I must be having brain issues today. Passing a pointer to a pointer would be canonRect(**r);, no? Same illegal indirection errors.
John Rudy
Wow; great edit! Thanks!
John Rudy
Just tested it; this is the one. Thank you for the very thorough explanation.
John Rudy
+7  A: 

Its also worth noting your variable 'struct rec temp' is going to go out of scope as soon as that method canonRect ends and you will be pointing to invalid memory.

Joel Cunningham
That's a good point I hadn't thought of. Good catch! Thanks!
John Rudy
+2  A: 

It sounds like you're confusing the 'dereference' operator (*) with the 'address of' operator (&).

When you write &r, that gets the address of r and returns a pointer to r (a pointer is just a memory address of a variable). So you really are passing a pointer into the function.

When you write *r, you are trying to dereference r. If r is a pointer, that will return the value that r is pointing to. But r isn't a pointer, it's a rect, so you'll get an error.

To make things more confusing, the * character is also used when declaring pointer variables. In this function declaration:

void canonRect(struct rect *r) {

r is declared to be a pointer to a struct rect. This is completely different from using * like this:

canonRect(*r);

In both cases, the * character means something completely different.

yjerem
+2  A: 

You might want to read up on the different ways parameters can (conceptually) be passed to functions. C is call-by-value, so when you pass your pointer to a rect into the function you are passing a copy of the pointer. Any changes the function makes to the value r directly (not indirectly) will not be visible to the caller.

If you want the function to provide the caller with a new struct then there are two ways of doing it: 1. you can return a rect: 2. you can pass a pointer to a pointer to a rect in:

The first way would be more natural:

struct rect* canonRect(struct rect* r)
{
  struct rect* cr = (struct rect*) malloc(sizeof(struct rect));
  ...
  return cr;
}

The second way would be:

void canonRect(struct rect** r)
{
  *r = (struct rect*) malloc(sizeof(struct rect));
}

and the caller would then use:

   canonRect(&r);

But the caller loses original pointer it had, and you would have to be careful not to leak structs.

Whichever technique you use, the function will need to allocate memory for the new struct on the heap using malloc. You can't allocate the space on the stack by just declaring a struct because that memory becomes invalid when the function returns.

Rob Walker
Rob, I think you got to the base of the issue, but your answer could be better if you included an example how the leak of the original r variable occurs. This seems to be an intro question, so inexperienced C coders might not catch the detail.
Dana the Sane
+2  A: 

First, K&R c does not have a concept of "references", just pointers. The & operator means "take the address of".


Secondly, the r in cannonRect() is a local variable, and is not the r in main(). Changing where the local r points does not effect the r in the calling routine.


Finally, as already noted the local struct rect is allocated on the stack, and goes out-of-scope at the close brace,

dmckee
True, but the bigger problem was that he needed to assign to *r.
Jonathan Leffler
A: 

2.Why might I get an illegal indirection compile-time error if I try to pass in a pointer to canonRect? (IE, if I had canonRect(*r); in main().)

Because, it is not the purpose of pointer.

plan9assembler