C pointer behavior not understood

Question

I have a simple structure, loc

typedef struct
{
  long blk;
  int offset;
} loc;

In a function avl_isnadd, it is passed in as:

int
avl_isnadd (old_loc, old_isn, isn)
loc *old_loc;
int old_isn, isn;
  {
    int next_isn;
    loc *this_loc;
    printf("\n{avl_isnadd} - old_loc-> blk = %d, old_loc->offset = %d\n", old_loc->blk, old_loc->offset);
    this_loc->blk = old_loc->blk;
    this_loc->offset = old_loc->offset;
    printf("\n{avl_isnadd} - this_loc->blk = %d, this_loc->offset = %d\n", this_loc->blk, this_loc->offset);
     next_isn = avl_isnget (this_loc);
     return next_isn;
}

and in avl_isnget, we have:

int
avl_isnget (myLoc)
loc *myLoc;
  {
    printf("\n{avl_isnget} - MyLoc->blk = %d, myLoc->offset = %d\n", myLoc->blk, myLoc->offset);
    return 0;
   }

The results on the console are:

{avl_isnadd} - old_loc-> blk = 1, old_loc->offset = 512

{avl_isnadd} - this_loc->blk = 1, this_loc->offset = 512


{avl_isnget} - MyLoc->blk = 1485457792, myLoc->offset = 512

What am I missing here? I don't see why avl_isnget should have a different value for myLoc->blk

you're not allocating any space for `this_loc`. As soon as you do `this_loc->blk = old_loc->blk;`, you're invoking undefined behavior. — yano, Sep 07 '17 at 15:55
So, if I define it as loc this_loc, how do I do the assignment and pass it to the function as a pointer? — John Wooten, Sep 07 '17 at 15:58
You need to allocate some memory. And get a newer C book – the old-style parameter list went out of fashion thirty years ago. — molbdnilo, Sep 07 '17 at 15:59
This is obsolete and strongly discouraged since over 28 years! Get a recent book, that will also help understanding pointers and memory allocation! — too honest for this site, Sep 07 '17 at 16:00
If you define it as `loc this_loc`, then you get the pointer of `this_loc` using `&`, thus: `&this_loc`. Are you making use of the C documentation? — lurker, Sep 07 '17 at 16:03
Turn up your warnings to pedantic levels and treat them as errors. There are a number of things wrong in this code, regardless of how legacy the parameter syntax is. [See here](http://coliru.stacked-crooked.com/a/a34d0efc05088c2c). — WhozCraig, Sep 07 '17 at 16:04
Thanks. Yes, I was reading the manual. I'm trying to revive a code written 30 years ago. During the revision, some errors crept in. A recent book doesn't help so much when the code is 30 years old. You have to read and understand the code as it was written then. Then, slowly migrate it to the new. Thanks so much for the help. It was obvious when pointed out. Try to understand that Sometimes we have to deal with what we have, not what we'd like it to be. Many of the "errors" by cutting to the essential features. — John Wooten, Sep 07 '17 at 16:05
Fundamentals such as using `&` to get the pointer of a variable haven't changed in 30 years. :) But I do feel your pain regarding revival of old, old code. As has already been mentioned, if you declare a pointer, then it must be assigned a valid address, either using `&` on another statically declared variable, or using dynamic allocation (`malloc`). That's all legacy stuff. — lurker, Sep 07 '17 at 16:09

score 0 · Answer 1 · answered Sep 07 '17 at 16:11

0

this_loc is a pointer which is pointing to invalid memory address. In fact this program should crash.

this_loc = old_loc should also work.

answered Sep 07 '17 at 16:11

Naresh

13
1

score 0 · Accepted Answer · answered Sep 07 '17 at 16:16

you're not allocating any space for this_loc, so as soon as you do this_loc->blk = old_loc->blk; you're invoking undefined behavior. You can either allocate space for this_loc in automatic storage or from the heap. I will demonstrate the automatic storage option, since I find that preferable considering the code presented (with updated syntax):

Automatic storage option:

int
avl_isnadd (loc *old_loc, int old_isn, int isn)
{
    int next_isn;
    loc this_loc; // don't make it a pointer. this declaration will allocate space for
                  // the struct in automatic storage (in many implementations,
                  // the stack) whose scope exists only in this function
    printf("\n{avl_isnadd} - old_loc-> blk = %ld, old_loc->offset = %d\n", old_loc->blk, old_loc->offset);  // printf uses the %ld specifier for a signed long 
    this_loc.blk = old_loc->blk;  // change the accessor operator from -> to .
    this_loc.offset = old_loc->offset;
    printf("\n{avl_isnadd} - this_loc.blk = %ld, this_loc.offset = %d\n", this_loc.blk, this_loc.offset);
    next_isn = avl_isnget (this_loc); // Simply pass the entire struct to the avl_isnget function.
    return next_isn;
    // this_loc goes out of scope (pops off the stack) and you're done with it
}

Then change your avl_isnget function to

int
avl_isnget (loc myLoc)
{
    // myLoc is now a local copy of the this_loc struct that you passed in.
    // Since this function doesn't modify the struct loc passed in, there's
    //no real point in passing in a pointer. A local copy will do just fine for printing
    printf("\n{avl_isnget} - MyLoc.blk = %ld, myLoc.offset = %d\n", myLoc.blk, myLoc.offset);
    return 0;
}

Another option would be to allocate space as loc* this_loc = malloc(sizeof(loc)); and go from there, but with the code presented, there's no reason to do that. Avoid memory management unless you have a good reason to do so.

C pointer behavior not understood

2 Answers2