Ever see anything like this?

[gbostock]

I'm working on some legacy code and came across something like this.
Anybody know exactly what the ramifications are to the memory if you do
something like this:

int somefunction (some arguments)
{
....

Mystructtype mystruct;

....

otherfunc(&mystruct); /* this one appears to load up the struct*/
....

free(&mystruct);

func3((void *}&mystruct, sizeof(Mystructtype)); /* func3 takes a void
** and mallocs memory

to set the pointer to point to! */

free(&mystruct);

....
} /* end somefunction */

Now this obviously is really bad, but just what happens? It seems to me
that there is a memory overwrite at the very least or possibly a memory
leak that only lasts as long as the function since the mystruct memory
is automatic anyway.

What would you do about it?

 

[Eric Sosman]

I'm working on some legacy code and came across something like this.
Anybody know exactly what the ramifications are to the memory if you do
something like this:

int somefunction (some arguments)
{
...

Mystructtype mystruct;

...

otherfunc(&mystruct); /* this one appears to load up the struct*/
...

free(&mystruct);

Right here, the program goes off the rails. The
memory for mystruct was not obtained from malloc() et
al., so attempting to release it with free() produces
undefined behavior.

It is pointless to ask "exactly what the ramifications
are" from the point of view of the C language: the language
says only that the consequences are undefined. As far as
the language is concerned, the program can do anything at
all; it is an outlaw in the sense that the laws no longer
apply to it.

The question may be answerable from the point of view
of a particular C implementation, but not in terms of
the language itself.

[...]
What would you do about it?

First, double- and triple-check that the code really,
truly does what your paraphrase says. Pay close attention
to indirection levels; note that the superficially similar

typedef struct s Mystructtype;
typedef Mystructptrtype *Mystructtype;

Mystructptrtype mystruct;
create (&mystruct);
...
destroy (&mystruct);

might be perfectly legitimate, given something like

void create (Mystructptrtype *ptr) {
*ptr = malloc(sizeof *ptr);
...
}

void destroy (Mystructptrtype *ptr) {
...
free (*ptr);
}

Examine the typedefs with particular care; when typedef is
used to create aliases for pointer types, the fact that
you're dealing with a multiple indirection can be less than
obvious. If it turns out that the code is in fact correct
but that the typedef names are muddying the waters, I'd
suggest getting rid of pointer typedefs and using just the
"base" types instead. Keep the typedef aliases for the
structs themselves, if you like (some people don't), but
get rid of the pointer typedefs based on them.

If it turns out that the code actually does what your
paraphrase says, you've got a problem: you need to read the
minds of the people who wrote the code and try to figure
out what in the world they were trying to accomplish. This
will require some ESP on your part, because the C doesn't
reveal their intentions -- as we've seen, the C as written
has no meaning and is therefore uninformative. Once you've
figured out what they were trying to do, rewrite the code so
it does that thing in some legitimate way.

Good luck!

 

[serrand]

I'm working on some legacy code and came across something like this.
Anybody know exactly what the ramifications are to the memory if you do
something like this:

int somefunction (some arguments)
{
....

Mystructtype mystruct; Mystructtype * ptrmystruct;

....

otherfunc(ptrmystruct); /* this one appears to load up the struct*/
....

free(&mystruct);

/* ==> segmentation fault... you can free variables mallocated (or callocated..) only as long as i know */
free (ptrmystruct);

func3((void *}&mystruct, sizeof(Mystructtype)); /* func3 takes a void
** and mallocs memory

func3((void *}&ptrmystruct, sizeof(Mystructtype));

to set the pointer to point to! */

free(&mystruct); free (ptrmystruct);

....
} /* end somefunction */

Now this obviously is really bad, but just what happens? It seems to me
that there is a memory overwrite at the very least or possibly a memory
leak that only lasts as long as the function since the mystruct memory
is automatic anyway.

What would you do about it?

Xavier

 

[Keith Thompson]

I'm working on some legacy code and came across something like this.
Anybody know exactly what the ramifications are to the memory if you do
something like this:

int somefunction (some arguments)
{
...

Mystructtype mystruct;

...

otherfunc(&mystruct); /* this one appears to load up the struct*/
...

free(&mystruct);

func3((void *}&mystruct, sizeof(Mystructtype)); /* func3 takes a void
** and mallocs memory

to set the pointer to point to! */

free(&mystruct);

...
} /* end somefunction */

Now this obviously is really bad, but just what happens? It seems to me
that there is a memory overwrite at the very least or possibly a memory
leak that only lasts as long as the function since the mystruct memory
is automatic anyway.

What would you do about it?

The first thing I'd do is look at the actual code rather than a
paraphrase of it. Since you re-typed a summary of the code rather
than copy-and-pasting an actual copy of it, it's impossible to be sure
what the original code actually looks like. I'm sure it doesn't
literally have the lines
int somefunction (some arguments)
and
func3((void *}&mystruct, sizeof(Mystructtype));
both of which are syntax errors.

*If* the code actually contains
free(&mystruct);
where mystruct is a declared object, it invokes undefined behavior.

C99 7.20.3.2p2:

The free function causes the space pointed to by ptr to be
deallocated, that is, made available for further allocation. If
ptr is a null pointer, no action occurs. Otherwise, if the
argument does not match a pointer earlier returned by the calloc,
malloc, or realloc function, or if the space has been deallocated
by a call to free or realloc, the behavior is undefined.

Undefined behavior means that anything can happen, including no
visible bad effects.

What I would do about it is fix it, if practical.

 

[gbostock]

/* ==> segmentation fault... you can free variables mallocated (or callocated..) only as long as i know */
free (ptrmystruct);

Actually it doesn't cause a segmentation fault. As others pointed out,
this results in undefined behavior and on the machine it's running on
it appears to do absolutely nothing. I did write a correction very
similar to the one that you came up with (before I saw your post).

I believe that the code that I'm looking at causes a memory leak
because instead of passing a (void) pointer to pointer, it passes (void
*)&mystruct. When this happens, malloc is looking at the pointer to the
structure and returns the pointer value of the memory it allocated into
the first bytes of the structure's memory. The rest of the program uses
the memory declared in the program, not the memory that was allocated
and since it's address has been written into the structure and
subsequently overwritten, the result is a memory leak.

So the code works, but I believe it causes a memory leak.