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

Re: [libvirt] [PATCH 5/6] Add an an internal API for emergency dump of debug buffer



On Mon, Mar 07, 2011 at 10:06:35AM +0000, Daniel P. Berrange wrote:
> On Mon, Mar 07, 2011 at 03:06:18PM +0800, Daniel Veillard wrote:
> > On Fri, Mar 04, 2011 at 08:53:55AM -0700, Eric Blake wrote:
> > > 
> > > snprintf is _not_ safe; it can call malloc.  We probably ought to use a
> > > manual decimal-to-string conversion loop instead.
> > 
> >   Even better I think a case on signum and outputting the signal
> > description is even more useful, and simpler.
> > 
> > > > +    buf[sizeof(buf) - 1] = 0;
> > > > +    virLogDumpAllFD(buf, strlen(buf));
> > > > +    snprintf(buf, sizeof(buf) - 1, "\n\n    ====== start of log =====\n\n");
> > > 
> > > Why snprintf here, rather than strcpy?
> > 
> >   pure lazyness.
> > 
> > I'm suggesting the following patch to fix those 2 issues, it get rids of
> > the temporary buffer which was used to compute the length, better done
> > in the logging routine directly. It also get rid of ret variable which
> > was never used :-\
> > I'm also removing the early virLogLen == 0 test because we should at
> > least log the fact we got the signal, even if the buffer may be empty.
[...]
> 
> ACK

  thanks, pushed !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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