Data_Make_Struct and ALLOC Considered Harmful?

Hi,

This is a follow-up of the thread with title "Data_Make_Struct
Considered Dangerous?".  Rather than making the thread larger and
larger, I try to start discussing the issue with a fresh start.

All this discussion is using ruby -v: ruby 1.6.7 (2002-03-01)
[i686-linux].

Data_Make_Struct is defined in ruby.h as

#define Data_Make_Struct(klass,type,mark,free,sval) (\
    sval = ALLOC(type),\
    memset(sval, 0, sizeof(type)),\
    Data_Wrap_Struct(klass,mark,free,sval)\
)

As we can see, the sequence is first ALLOC() is called, and then the
memory is set to zero by using memset().  ALLOC() is just an alias of
xmalloc(), which is just an alias of ruby_xmalloc().  ruby_xmalloc()
is defined in gc.c as

void *
ruby_xmalloc(size)
    long size;
{
    void *mem;

    if (size < 0) {
	rb_raise(rb_eNoMemError, "negative allocation size (or too big)");
    }
    if (size == 0) size = 1;
    malloc_memories += size;

    if (malloc_memories > GC_MALLOC_LIMIT) {
	rb_gc();
    }
    RUBY_CRITICAL(mem = malloc(size));
    if (!mem) {
	rb_gc();
	RUBY_CRITICAL(mem = malloc(size));
	if (!mem) {
	    if (size >= 10 * 1024 * 1024) {
		rb_raise(rb_eNoMemError, "tried to allocate too big memory");
	    }
	    mem_error("failed to allocate memory");
	}
    }

    return mem;
}

As we can see, rb_gc() may be called before the memory is returned. 
rb_gc(), in turn, calls all the rb_gc_mark() functions.  Therefore,
one particular issue is:

"Unless the C programmer is very careful, when using ALLOC it is very
possible that the marking function is called before a memory space is
properly initialized".

Suppose we assume that the marking functions are really proper, i.e.,
they always check the state of each memory before rb_gc_mark is called
such as:

    if (ptr) rb_gc_mark (...);

Suppose also we have a struct such as

    typedef struct
    {
        struct_A *data1;
        struct_B *data2;
    } struct_C;

which is involved in some rb_gc_mark() because struct_C is part of
(but not direct) internal data of some class.  Now, the following code
may fail:

    ....
    ptr = ALLOC (struct_C);
    ptr->data1 = ALLOC (struct_A);    /* may fail here because of
rb_gc() */
    ptr->data2 = ALLOC (struct_B);

but the following code will never fail:

    ....
    ptr = SAFE_MALLOC (struct_C);
    ptr->data1 = SAFE_MALLOC (struct_A);
    ptr->data2 = SAFE_MALLOC (struct_B);

where SAFE_MALLOC is just a wrapper of malloc() that deals with
out-of-memory.  The only way to solve the ALLOC problem is to do
"double-initialization":

    ....
    ptr = ALLOC (struct_C);
    ptr->data1 = NULL;
    ptr->data2 = NULL;    /* or just use one memset when it is proper
*/
    ptr->data1 = ALLOC (struct_A);
    ....

We see that the initialization to zero by memset in Data_Make_Struct
does not really help here.  It only helps when struct_C is a direct
data part of some class, in which case it was created using

    obj = Data_Make_Struct (cClass, struct_C, ..., ..., ptr);

But when ptr is some other level deeper inside obj, the above problem
occurs.

Therefore, rather than dealing with this extra issue, why don't we
simply not use ALLOC()?  Yes, when used properly, nothing is harmful. 
But just like the GO TO statement, it creates a potential pitfall for
the unaware C programmer.  This stems from the fact that structured
programming was created to minimize the "jumping around" problem with,
for example, the GO TO statement.  But ALLOC(), with its rb_gc(), with
its rb_gc_mark(), follows some "jumping around" pattern too.

Not using ALLOC also has this benefit:  Let Ruby manage its own memory
and let the C part manage its own memory.  There is no need for the C
part to be involved in Ruby memory management.  (If really necessary,
the C part can call rb_gc() explicitly.)  There was a comment in the
previous thread that by not using ALLOC, the Ruby heuristic of when to
start the next GC might be way off; but isn't that this problem also
occurs when the C part is not there?  (For example, with
GC_MALLOC_LIMIT of 8000000, isn't that there is always a potential of
Ruby of wasting 7999999 bytes of memory, whether it is pure Ruby or a
mix of Ruby and C?  If the pure Ruby already has some mechanism of
getting rid the wasted 7999999 bytes, then there is no need for the C
part to interfere.)

Finally, any Ruby function that allocates some memory (such as
rb_str_new2) has the potential of invoking rb_gc_mark().  I don't see
any other way around this problem.  However, when we don't use
ALLOC(), we reduce the possibility of error.  I guess it is the same
as the GO TO statement.  When used properly, it is not dangerous; and
even when it is removed, a language like C is still "dangerous"
because of pointers.  The removal (or disuse) of the GO TO statement
does not make a language 100% safe, but it seems that now people do
not use GO TO very-very often anymore because people have understood
the issue.

Therefore, to summarize:

    1) Use malloc() (or some SAFE_MALLOC()) to allocate memory; don't
use ALLOC
    2) Because of 1), use Data_Wrap_Struct; don't use Data_Make_Struct
    3) Be aware that before calling any Ruby function that may
allocate memory (such as Data_Wrap_Struct() and rb_str_new2()), all
memory should be proper (double-initialization may be required)
    4) In the absence of 3), using malloc() a C programmer can proceed
as usual: allocate memory and then initialize it (no need for
double-initialization)

Regards,

Bill

A particular note to Guy: If you have not understood the issue : don't
response.
We have no need for a technical-less "it is stupid" comment.

============================================================================

Data_Make_Struct() is *not* dangerous. The goal of Data_Make_Struct()
is
 to call ALLOC(), initialize all members of the struct to zero then
call
 Data_Wrap_Struct()

 If you have not understood this : don't use it.



Guy Decoux