For review: GSS memory allocation patch

Kevin Wasserman krwasserman at hotmail.com
Fri Oct 14 12:16:33 EDT 2011


That all sounds reasonable to me, though personally I would argue
that if the contract is to always completely destroy the data list,
I would keep the extra indirection and set it to NULL before
completion.  Actually, previously, the list _elements_ were all
destroyed, but not the list itself; would you actually prefer that
behavior?

-----Original Message----- 
From: Greg Hudson
Sent: Friday, October 14, 2011 11:59 AM
To: Kevin Wasserman
Cc: Sam Hartman ; krbdev at mit.edu
Subject: Re: For review: GSS memory allocation patch

On 10/14/2011 09:27 AM, Kevin Wasserman wrote:
> "gssalloc memory management for gss_buffer_set."
> https://github.com/hartmans/kfw-updates/commit/5a4583e13eb5ac1597dff10abee0597bb714f4c6

I'm not fond of the kg_data_list_to_buffer_set_nocopy contract after
this change.  What I'd like to see is:

* Remove the _nocopy suffix, since it's a lie on one platform (and also
the kg_ prefix, for unrelated reasons).

* The helper should destroy the data list on success or failure.  The
current contract of "totally destroys on success, maybe partially
destroys on failure" makes it hard to verify correctness.  (Before the
allocation change, the contract was "destroy on success, leave alone on
failure", which was reasonable.)

* Remove the extra layer of indirection for the data list.

A previously existing but related bug is that the caller should not fail
when the output attribute parameter is NULL. 




More information about the krbdev mailing list