[hipl-dev] Re: [Branch ~hipl-core/hipl/trunk] Rev 5156: Remove unnecessary NULL checks before free() invocations on pointers.
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Mon, 15 Nov 2010 17:25:07 +0100
On Mon, Nov 15, 2010 at 04:23:24PM +0100, Stefan Götz wrote:
> > === modified file 'firewall/conntrack.c'
> > --- firewall/conntrack.c 2010-10-27 11:27:48 +0000
> > +++ firewall/conntrack.c 2010-11-15 15:12:49 +0000
> > @@ -496,29 +496,18 @@
> > */
> > static void free_hip_tuple(struct hip_tuple *hip_tuple)
> > {
> > - if (hip_tuple) {
> > - if (hip_tuple->data) {
> > -
> > - // free keys depending on cipher
> > - if (hip_tuple->data->src_pub_key && hip_tuple->data->src_hi) {
> > - if (hip_get_host_id_algo(hip_tuple->data->src_hi) ==
> > HIP_HI_RSA) {
> > - RSA_free((RSA *) hip_tuple->data->src_pub_key);
> > - } else {
> > - DSA_free((DSA *) hip_tuple->data->src_pub_key);
> > - }
> > - }
> > -
> > - if (hip_tuple->data->src_hi) {
> > - free(hip_tuple->data->src_hi);
> > - }
> > -
> > - free(hip_tuple->data);
> > - hip_tuple->data = NULL;
> > + // free keys depending on cipher
> > + if (hip_tuple->data->src_pub_key && hip_tuple->data->src_hi) {
> > + if (hip_get_host_id_algo(hip_tuple->data->src_hi) == HIP_HI_RSA) {
> > + RSA_free((RSA *) hip_tuple->data->src_pub_key);
> > + } else {
> > + DSA_free((DSA *) hip_tuple->data->src_pub_key);
> > }
> > -
> > - hip_tuple->tuple = NULL;
> > - free(hip_tuple);
> > }
> > +
> > + free(hip_tuple->data->src_hi);
> > + free(hip_tuple->data);
> > + free(hip_tuple);
> > }
>
> I think you changed the semantics of this function. Before your change,
> free_hip_tuple() gracefully handled a NULL hip_tuple and a NULL
> hip_tuple->data.
> Now, they trigger SEGFAULTs.
Yup, the change is bad, I'll uncommit it.
> Maybe there other such cases in that commit, I didn't look further.
I think there are others, I'll redo the commit properly later.
Thanks for noticing the breakage.
Diego
Other related posts:
