Re: [Qemu-devel] [PATCH 27/40] char: move QIOChannel-related in char-io.h

[Marc-André Lureau]

Hi

----- Original Message -----
> On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> 
> Grammar in subject is a bit terse; maybe:
> 
> char: move QIOChannel-related stuff to char-io.h
> 
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  chardev/char-io.h     |  24 +++++++
> >  chardev/char-io.c     | 168
> >  ++++++++++++++++++++++++++++++++++++++++++++++++
> >  chardev/char.c        | 174
> >  +-------------------------------------------------
> >  chardev/Makefile.objs |   1 +
> >  4 files changed, 194 insertions(+), 173 deletions(-)
> >  create mode 100644 chardev/char-io.h
> >  create mode 100644 chardev/char-io.c
> > 
> > diff --git a/chardev/char-io.h b/chardev/char-io.h
> > new file mode 100644
> > index 0000000000..ea559fd124
> > --- /dev/null
> > +++ b/chardev/char-io.h
> > @@ -0,0 +1,24 @@
> > +#ifndef CHAR_IO_H
> > +#define CHAR_IO_H
> 
> Must... resist... the temptation to repeat myself!

Ok, I'll just copy the original copyright from qemu-char.c in all the files I 
created in the series.

> 
> > +
> > +#include "qemu/osdep.h"
> 
> .h files should NOT include osdep.h; since all .c files included it
> before any other .h file, then all things in osdep.h are already in
> scope at the point the .h file starts.

ok

> 
> 
> > +static gboolean io_watch_poll_prepare(GSource *source,
> > +                                      gint *timeout_)
> > +{
> 
> Why the weird spelling of timeout_ ?  Maybe timeout_unused is better?
> 
> 
> > +++ b/chardev/char.c
> 
> > -static gboolean io_watch_poll_prepare(GSource *source,
> > -                                      gint *timeout_)
> 
> Then again, it's code motion.  Again, up to you if you want to tweak
> style things during code motion, or split them into a general cleanup
> patch separately.

yeah, weird style, any idea why Anthony wrote it that way in commit 96c638477?

I guess I may just drop the _ during the move