On Thu, Mar 04, 2010 at 01:31:33PM -0500, Dave Allan wrote: > On 03/03/2010 07:20 PM, Ed Swierk wrote: > >On Wed, Mar 3, 2010 at 2:57 PM, Dave Allan<dallan@xxxxxxxxxx> wrote: > >>Although I use goto a lot, I generally try to avoid multiple labels > >>within a > >>function, just because I think it gets out of hand really quickly. > >>Although > >>it's a slightly more invasive patch, would you refactor the code to look > >>something like what I've attached? I haven't even compile tested it as > >>I'm > >>running late, but that's the idea. > > > >Is there a piece of code in libvirt that exemplifies the preferred > >error handling style? (http://libvirt.org/hacking.html doesn't cover > >this issue, as far as I can tell.) Just in the very small part of > >libvirt I've hacked on recently I've found a variety of styles, > >including > > Agreed that we should add a statement to the hacking guide. My > preferences are as follows. > > >- pair every allocation with a goto label that frees the allocation > >and all the earlier ones, and goto the appropriate label on error > > I like Robert Love's description of this style at the very end of the > thread at: > > http://kerneltrap.org/node/553/2131 > > I like this style, but my impression is that generally the libvirt > community prefers to have a single label that frees everything, perhaps > conditionally on error, unless it's absolutely necessary to have > multiple labels. If the cleanup code only involves free'ing memory, then having multiple labels is complete overkill. VIR_FREE() and every function named XXXFree() in libvirt is required to handle NULL as its arg. Thus you can safely call free on all the variables even if they wre not yet allocated (yes they have to have been initialized to NULL). This is much simpler & clearer than having multiple gotos Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list