On Sun, 2003-07-27 at 13:37, Derek Atkins wrote:
> Matthew,
>
> Matthew Vanecek <[EMAIL PROTECTED]> writes:
>
> > After discusing with Derek a bit, here's the patch that corrects bug
> > #116546. This is against head; I've not backported to 1.8.x yet.
>
> A couple comments:
>
> 1) I don't think you want to g_free() the blist entries. This could
> be a big problem with double-freeing data. In particular:
>
> + for (node = be->blist; node; node = node->next) {
> + g_free(node->data);
> + node->data = NULL;
> + }
>
> I don't think you want this... (in either place you have it).
> Granted, I'm not sure how the blist is created, but I'm pretty sure
> this is not the right way to clean it up. ;)
>
I'm not sure if we should free the data nodes or not. It doesn't seem
to cause any problems, but that's not always a sure indicator of
appropriateness, is it? =P I tried to reason through it a bit and
somehow decided they should be freed. I think my thought was that since
we are calling g_list_free(blist), and that g_list_free() doesn't free
dynamically allocated node->data, we should do it explicitly. The
original (as you can see), simply called g_list_free() on be->blist,
which I guess is OK to leave that way...
FWIW, g_free does allow a NULL argument.
> 2) This is a minor UI message bug:
>
> + if (1 < be->nest_count) {
> + LEAVE("be->nest_count < 1: %d", be->nest_count);
>
> The message is reversed. This is checking whether be->next_count > 1
>
> (FWIW, I absolutely despise this style, for this exact reason!)
>
> -derek
--
Matthew Vanecek
perl -e 'print $i=pack(c5,(41*2),sqrt(7056),(unpack(c,H)-2),oct(115),10);'
********************************************************************************
For 93 million miles, there is nothing between the sun and my shadow except me.
I'm always getting in the way of something...
signature.asc
Description: This is a digitally signed message part
_______________________________________________ gnucash-devel mailing list [EMAIL PROTECTED] http://www.gnucash.org/cgi-bin/mailman/listinfo/gnucash-devel
