C Can't Handle Pointers

[csteverun]

I could really use some help here. I can't figure out what's the problem is, so I've assumed it's all C's fault.

I have a function:

pf_plot32(Uint32 data[], Uint32 dl, pfplot pattern[], Uint8 pl, Uint32 pr, struct pf_path32 *path)

..Which takes a pointer to a structure:

struct pf_point32 {
  Uint32 x;
  Uint32 y;
};
struct pf_path32 {
  Uint32 length;
  struct pf_point32 *points[];
};

..And then plots points to it dynamically using this call/function:

...
path->points[length] = (struct pf_point32*)pf_allocate(sizeof(struct pf_point32));
path->points[length]->x = x;
path->points[length]->y = y;
length++;
...
void *pf_allocate (size_t size) {
    void *p;
    p = malloc(size);
    if (p == NULL) {
      sprintf(str, "Out of memory allocating size of %d\n", size);
      pf_log(str);
    }
    return p;
}

Now the function worked perfectly making pretty little paths for me, until I tried to use it twice in a row. When I use it again, for no reason that I can even begin to comprehend, it changes the FIRST structure's length (not even the one that was passed to it!) to a crazy-num, something like "4153952" instead of the real length of something like "4".

How could this function be changing a structure that's not even given to it? I tracked down a possible location of the problem, but can't find the source. The evil backwards change seems to happen right after this line in the pf_plot32() function:

path->points[length] = (struct pf_point32*)pf_allocate(sizeof(struct pf_point32));

Why would allocating memory for one structure change an unrelated value (length) of an unrelated structure? Keep in mind, it's messing up the length of the PREVIOUS pf_path32 that was plotted, not the one that's currently being passed to it (which comes out fine).

Here is an example:

struct pf_path32 boundrypath;
    pf_plot32(boundrydata, 2, boundrypattern, 11, 1, &boundrypath);
sprintf(str, "boundrypath.length = %d", boundrypath.length); pf_log(str);

    struct pf_path32 blockpath;
    pf_plot32(blockdata, 2, blockpattern, 11, 1, &blockpath);

sprintf(str, "boundrypath.length = %d", boundrypath.length); pf_log(str);
sprintf(str, "boundrypath.length is now = %d", boundrypath.length); pf_log(str);

..Outputs:

boundrypath.length = 4
blockpath.length = 4
boundrypath.length is now = 4153952

Thanks for reading my code. I hope someone can find a solution. Otherwise, I'm going to have to find a programming language that does what it's supposed to! :) Thanks again,
Steve

[csteverun]

I should add that length is set at the end of pf_plot32() like this:

path->length = length;

[csteverun]

Here is the full source is anyone needs it:
http://groovyrings.com/temp/pitchblocks.c
http://groovyrings.com/temp/pf.h

[Ancient Dragon]

>>so I've assumed it's all C's fault.
Never make that assumption because you will ALWAYS be wrong.

>>path->points[length] = (struct pf_point32*)pf_allocate(sizeof(struct pf_point32));
function pf_plot32()
That is trashing memory because points is a pointer array of unspecified length

I am supprised that you have coded all that but have never been told to NEVER EVER put executable code in header files. If you need to put executable code in a file other then the main program *.c file then create another *.c file, compile the two separetely and link them both together at the end. How to do that depends on the compiler you are using because they all do it differently.

[csteverun]

I apologize, I'm a C newbie. I thought that's what header files were for. Can I just change the H file to a C file (i.e. "#include pf.c" instead of "#include pf.h")? And is there a technical reason for this, or is it just "good form"?

As for points, the path length will always be different depending on what data and what pattern is ran through it. That's why I'm using dynamical memory allocation. This has always worked in the past. If this is wrong, how is one supposed to manage a pointer array of unknown length?

Thanks for your help.

[csteverun]

Yippie! Thanks AD, I got it working by just adding a length to the pointer array as you said.

Initially I thought this was wrong - a waste of space, that C would index the allocated pointers to the array as they were set - but then I remembered that C can't do anything without being told. Also, a pointer must only take like a couple bytes of space, so even if most of the array is wasted it's probably only like a couple kilobytes of memory per 1000 pointers in the array.

But I would still like to know about the C vs H issue if you get a chance. Thanks again.

[Ancient Dragon]

Header files are often included in two or more program units (*.c files). So if you put executable code in the header file then the compiler will spew out duplicate declaration errors when attempting to link then. By convention (and everybody with more than 1 day's programming experience) will only put structure declarations, typedefs, defines and the like in header files -- and nothing more. Many programs and libraries consist of hundreds of *.c and .h files and trying to compile them all in one *.c file would be a nightmare for even the most experienced programmers.

In your program you should put the executable code in a different *.c file and leave the structures and defines where they are. Now your project will have two *.c files and one *.h file. Include the *.h file at the top of each *.c file, compile both *.c files then link them together as one executable program.

You did not say what compiler you are using so I can't help you any more with that part.

[Ancient Dragon]

you could change points to be double star then allocate both dimensions

struct pf_path32 {
  Uint32 length;
  struct pf_point32 *points[];
};
path->points = malloc((path->length+1) * sizeof(char *));
// now you can use it like without wasted space

You will need the +1 in the above because you reference like this:

path->points[length]

If length is the same as path->length then you will be indexing out-of-bounds because arrays are numbered starting with 0, so length element will not exist.

[csteverun]

I see, yes, that's a good solution. But I think I'll leave like it is because it allocates while it's actually building the path. If I did it like that, I'd need to add code to at least scan the pattern and find out how many plots (points) there will be.