[Spice-devel] [vdagent-win PATCH v8 1/5] Move image handling to a separate file

[Frediano Ziglio]

> 
> On Fri, Jul 21, 2017 at 08:25:30AM -0400, Frediano Ziglio wrote:
> > > 
> > > On Wed, Jul 19, 2017 at 09:42:39AM +0100, Frediano Ziglio wrote:
> > > > This will make easier to change code that handle images.
> > > > 
> > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> > > > ---
> > > >  Makefile.am         |  2 ++
> > > >  vdagent/image.cpp   | 88
> > > >  +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  vdagent/image.h     | 48 +++++++++++++++++++++++++++++
> > > >  vdagent/vdagent.cpp | 57 +++++-----------------------------
> > > >  4 files changed, 146 insertions(+), 49 deletions(-)
> > > >  create mode 100644 vdagent/image.cpp
> > > >  create mode 100644 vdagent/image.h
> > > > 
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index b60a718..868199e 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -42,6 +42,8 @@ vdagent_SOURCES =			\
> > > >  	vdagent/vdagent.cpp		\
> > > >  	vdagent/as_user.cpp		\
> > > >  	vdagent/as_user.h		\
> > > > +	vdagent/image.cpp		\
> > > > +	vdagent/image.h			\
> > > >  	$(NULL)
> > > >  
> > > >  vdagent_rc.$(OBJEXT): vdagent/vdagent.rc
> > > > diff --git a/vdagent/image.cpp b/vdagent/image.cpp
> > > > new file mode 100644
> > > > index 0000000..960058d
> > > > --- /dev/null
> > > > +++ b/vdagent/image.cpp
> > > > @@ -0,0 +1,88 @@
> > > > +/*
> > > > +   Copyright (C) 2013-2017 Red Hat, Inc.
> > > > +
> > > > +   This program is free software; you can redistribute it and/or
> > > > +   modify it under the terms of the GNU General Public License as
> > > > +   published by the Free Software Foundation; either version 2 of
> > > > +   the License, or (at your option) any later version.
> > > > +
> > > > +   This program is distributed in the hope that it will be useful,
> > > > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > +   GNU General Public License for more details.
> > > > +
> > > > +   You should have received a copy of the GNU General Public License
> > > > +   along with this program.  If not, see
> > > > <http://www.gnu.org/licenses/>.
> > > > +*/
> > > > +
> > > > +#include <spice/macros.h>
> > > > +
> > > > +#include "vdcommon.h"
> > > > +#include "image.h"
> > > > +
> > > > +#include "ximage.h"
> > > > +
> > > > +typedef struct ImageType {
> > > > +    uint32_t type;
> > > > +    DWORD cximage_format;
> > > > +} ImageType;
> > > > +
> > > > +static const ImageType image_types[] = {
> > > > +    {VD_AGENT_CLIPBOARD_IMAGE_PNG, CXIMAGE_FORMAT_PNG},
> > > > +    {VD_AGENT_CLIPBOARD_IMAGE_BMP, CXIMAGE_FORMAT_BMP},
> > > > +};
> > > > +
> > > > +static DWORD get_cximage_format(uint32_t type)
> > > > +{
> > > > +    for (unsigned int i = 0; i < SPICE_N_ELEMENTS(image_types); i++) {
> > > > +        if (image_types[i].type == type) {
> > > > +            return image_types[i].cximage_format;
> > > > +        }
> > > > +    }
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +HANDLE get_image_handle(const VDAgentClipboard& clipboard, uint32_t
> > > > size,
> > > > UINT&)
> > > 
> > > For what it's worth, I would have just removed that UINT& arg for now.
> > > 
> > > Christophe
> > > 
> > 
> > The interface was written that way
> 
> The interface? This is a helper you inroduce in this commit, surely it's
> possible to define it one way now, and improve it in future commits when
> needed.
> 
> Christophe
> 

As I said I wrote the interface that way.
Why to improve if still consistent?
So I had to spend more time to design it?

Frediano