Re: [PATCH spice-gtk v2] spice-channel: Factor out code for storing caps

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

 



Hi,

On Fri, Jun 30, 2017 at 12:12:06PM +0200, Christophe Fergeau wrote:
> On Fri, Jun 30, 2017 at 10:37:36AM +0200, Victor Toso wrote:
> > Hi,
> > 
> > On Fri, Jun 30, 2017 at 10:14:27AM +0200, Pavel Grunt wrote:
> > > Signed-off-by: Pavel Grunt <pgrunt@xxxxxxxxxx>
> > > ---
> > > v2 per Christophe's suggestion:
> > > use GArray as parameter
> > > ---
> > >  src/spice-channel.c | 34 ++++++++++++++++++++--------------
> > >  1 file changed, 20 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/src/spice-channel.c b/src/spice-channel.c
> > > index b8cf19c..4c3db9d 100644
> > > --- a/src/spice-channel.c
> > > +++ b/src/spice-channel.c
> > > @@ -1884,11 +1884,25 @@ cleanup:
> > >  #endif /* HAVE_SASL */
> > >  
> > >  /* coroutine context */
> > > +static void store_caps(const uint8_t *caps_src, const GArray *caps_dst)
> > > +{
> > > +    uint32_t *caps;
> > > +    guint i;
> > > +
> > > +    caps = &g_array_index(caps_dst, uint32_t, 0);
> > > +    memcpy(caps, caps_src, caps_dst->len * sizeof(uint32_t));
> > > +    for (i = 0; i < caps_dst->len; i++, caps++) {
> > > +        *caps = GUINT32_FROM_LE(*caps);
> > > +        SPICE_DEBUG("\t%u:0x%X", i, *caps);
> > > +    }
> > > +}
> > 
> > Yes, better
> 
> I don't think we'll be able to know from the debug message whether it's
> a channel or common cap though. Maybe add a const char *debug_prefix to
> the call now that you dropped a few args?
> 
> Christophe

From that debug alone, you are right but Pavel has added a debug prior
the function call:

 CHANNEL_DEBUG(channel, "got remote common caps:");
 store_caps(...);

That should be enough, no?

Cheers,

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]