Re: [libvirt] [PATCH 1/3] tools: rename console.[ch] to virsh-console.[ch] and fix coding style

On 29.08.2013 18:36, Peter Krempa wrote: > --- > cfg.mk | 2 +- > po/POTFILES.in | 2 +- > tools/Makefile.am | 2 +- > tools/{console.c => virsh-console.c} | 73 ++++++++++++++++++++++-------------- > tools/{console.h => virsh-console.h} | 4 +- > tools/virsh-domain.c | 2 +- > tools/virsh.c | 2 +- > 7 files changed, 52 insertions(+), 35 deletions(-) > rename tools/{console.c => virsh-console.c} (90%) > rename tools/{console.h => virsh-console.h} (91%) >

> diff --git a/tools/console.c b/tools/virsh-console.c > similarity index 90% > rename from tools/console.c > rename to tools/virsh-console.c > index 6c24fcf..debf12c 100644 > --- a/tools/console.c > +++ b/tools/virsh-console.c > @@ -1,7 +1,7 @@ > /* > - * console.c: A dumb serial console client > + * virsh-console.c: A dumb serial console client > * > - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc. > + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc. > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public Not shown in the context, but there should be 'Authors:' line just above Dan's name.

> @@ -171,9 +192,8 @@ virConsoleEventOnStream(virStreamPtr st, > > avail = con->terminalToStream.length - con->terminalToStream.offset; > if (avail > 1024) { > - if (VIR_REALLOC_N(con->terminalToStream.data, > - con->terminalToStream.offset + 1024) < 0) > - {} > + ignore_value(VIR_REALLOC_N(con->terminalToStream.data, > + con->terminalToStream.offset + 1024)); > con->terminalToStream.length = con->terminalToStream.offset + 1024; I don't think this is quite right. If the VIR_REALLOC fails, why are we increasing the .lenght? I know this is pre-existing, but if we are touching this we should fix this.

> } > }

> diff --git a/tools/console.h b/tools/virsh-console.h > similarity index 91% > rename from tools/console.h > rename to tools/virsh-console.h > index 46255b8..96ef235 100644 > --- a/tools/console.h > +++ b/tools/virsh-console.h > @@ -1,7 +1,7 @@ > /* > - * console.c: A dumb serial console client > + * virsh-console.h: A dumb serial console client > * > - * Copyright (C) 2007, 2010, 2012 Red Hat, Inc. > + * Copyright (C) 2007, 2010, 2012-2013 Red Hat, Inc. > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public Not shown in the context, but there should be 'Authors:' line just above Dan's name.

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index c87cf6a..60eed51 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -41,7 +41,7 @@ > #include "virbuffer.h" > #include "c-ctype.h" > #include "conf/domain_conf.h" > -#include "console.h" > +#include "virsh-console.h" This should go a few lines down, just before the line:

#include "virsh-domain-monitor.h" > #include "viralloc.h" > #include "vircommand.h" > #include "virfile.h" > diff --git a/tools/virsh.c b/tools/virsh.c > index 2f04e6a..0cc9bdd 100644 > --- a/tools/virsh.c > +++ b/tools/virsh.c > @@ -57,7 +57,6 @@ > #include "virerror.h" > #include "base64.h" > #include "virbuffer.h" > -#include "console.h" > #include "viralloc.h" > #include "virxml.h" > #include <libvirt/libvirt-qemu.h> > @@ -73,6 +72,7 @@ > #include "virtypedparam.h" > #include "virstring.h" > > +#include "virsh-console.h" > #include "virsh-domain.h" > #include "virsh-domain-monitor.h" > #include "virsh-host.h" > I've pointed a few issues but I believe that you will fix them right so I don't need to see a v2. ACK Michal