Re: [PATCH] Detect heap allocation failure; factor out some duplication.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 29, 2007 at 07:35:47PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> 
> > On Wed, Nov 28, 2007 at 02:18:22PM +0100, Jim Meyering wrote:
> >> I spotted a few unchecked heap allocations (strdup, malloc, calloc)
> >> in qemud.c and have fixed it so such failure evokes a proper diagnostic
> >> rather than e.g., a segfault.
> >
> > Yep, good stuff.
> >
> >> I've also arranged to free some of the memory upon failure,
> >> but not all (see comments for why).
> >>
> >> In spite of those additions, this patch factors out enough
> >> duplication that the net change is to remove a few lines from the
> >> file.  Although in general I prefer to factor things out into
> >> separate functions, in this case, it seemed better to use macros.
> >
> > I really don't like the macros, particularly when the macro definitions
> > are inline to the function. I'd prefer to see these helpers as separate
> > functions. The compiler is perfectly able to decide whether to inline
> > the code or not & it makes it more friendly to edit & read when it is
> > using separate functions.
> 
> I've rewritten to use fewer macros.
> In the process, I did a little testing, and will
> add an automated test to give this code some coverage
> in a separate patch.

Ok, this looks good to me now.

Can you commit the code changes, but leave out the config file changes - I've
re-written/structured major parts of the config file in my SASL patches, so
I'll just include the few typos you found in my updated patches.


Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]