[688] in Kerberos_V5_Development
Sommerfeld: memory allocation changes for krb5.
jtkohl@ATHENA.MIT.EDU (jtkohl@ATHENA.MIT.EDU)
Wed Apr 24 17:40:22 1991
Me: I'd like to put this to krbdev for discussion; any objections on my
posting your message directly?
Bill:
Go ahead; I'll trust the discretion of the people on the list to not
leak this back to Chelmsford until it's too late :-) .
From Bill:
Ted and I talked on Monday night about some fairly drastic changes to
the krb5 implementation which would significantly improve its memory
allocation behavior and significantly reduce the chance of memory
leaks due to programming errors.
In my "spare time" (i.e., this is something I'm doing on my own, and
not as an officially sanctioned project -- this is one of those things
which it is better to get forgiveness later than permission first),
I've taken a copy of the virgin alpha4 sources, and started to play
with these ideas. The bad news is that just about every source file
needs to be touched.
The good news is that most of the changes are fairly brainless and go
pretty quickly, and they result in a fairly significant reduction in
the complexity of the code. I've done the data structure arrangement
already and I'm in the process of walking through the libraries
getting them to build again. (I've done all of lib/free, and am a
third of the way through lib/krb after a couple of hours of work last
night).
Here's the approach I'm taking:
- In data structures: if it isn't an array, it should be inline.
This means that krb5_principal becomes an array of krb5_data, not an
array of pointers to krb5_data.
- krb5_free_<foo> changes to krb5_<foo>_free, and only frees the
contents, not the container; it is still passed a pointer to the
container. All pointers in the container are zeroed after being
freed, so that you can call krb5_<foo>_free on an object more than
once. Think of this routine like a C++ destructor.
- krb5_copy_<foo> changes to krb5_<foo>_copy, and takes two <foo> *'s
as arguments. In general (except for arrays) the caller is
responsible for allocating space for the top level of structure.
krb5_<struct foo>_copy() is just a call to krb5_<field type>_copy for
each field of the structure (except where the copy is trivial). I'm
thinking of changing the order of arguments to be (to, from), not
(from, to), for consistency with assignment statements, str*cpy,
memcpy, and memmove.
- add krb5_zero_<foo> macros, which just memset (&<foo>, 0,
sizeof(foo)); (on architectures where the bit-encoding of null
pointers is not all zeros, this would have to be "assign 0 to all
fields").
- add krb5_data_alloc (int, krb5_data *)
to fill in the length field, and krb5_data_extend (krb5_data *, int)
to "realloc" the data pointed at by a krb5_data.
- ditto for krb5_enc_data, except that the allocator/extender takes
an eblock as an argument (to fill in the etype field automagically).
- add krb5_data_clear to bzero the data and then free it.
- entry and cleanup code turns into:
krb5_frob_stuff (in1, in2, out1, out2)
...;
krb5_<bar> *out1;
krb5_<bar1> *out2;
krb5_<foo> local1;
krb5_<foo1> local2;
krb5_<foo>_zero (&local1);
krb5_<foo1>_zero (&local2);
krb5_<bar>_zero(out1);
krb5_<bar1>_zero(out2);
/* do real work */
retval = ....
if (retval) goto out;
retval = ....
if (retval) goto out;
retval = ...
if (retval) goto out;
out:
krb5_<foo>_free(&local1);
krb5_<foo1>_free(&local2);
if (retval) {
krb5_<bar>_free (out1);
krb5_<bar1>_free (out2);
}
return retval;
}
It's really nice on aesthetic grounds to avoid using gotos, but the
current way of doing cleanup with the cleanup_foo() macros is really
hard to read and hard to maintain when you add another variable to
clean up.
----
I'm willing to help out merging this stuff into the master sources at
an appropriate time, hopefully before beta; given that it's all based
off of the alpha4 snapshot, it should be fairly simple to merge using
automated tools (like rcsmerge):
- for each file that I change, check it in as a branch off of
the alpha4 version.
- use rcsmerge to generate a merged version.
- look for warnings from the merge, and clean them up manually.
- fix anything left over that doesn't build.
- Bill
