On Fri, 2017-07-14 at 11:58 +0200, Victor Toso wrote:
> Hi,
>
> On Thu, Jul 13, 2017 at 11:52:21AM -0500, Jonathon Jongsma wrote:
> > On Fri, 2017-06-30 at 12:51 +0200, Victor Toso wrote:
> > > From: Victor Toso <me@xxxxxxxxxxxxxx>
> > >
> > > Changing the name from clear_streams() to streams_finalize() to
> > > better
> > > match streams_check_init().
> >
> > Since I was not really in favor of the new factored-out
> > streams_check_init() function, the consistency issue maybe isn't as
> > relevant, but I'm not opposed to a better name here.
> > I'm not sure that I like _finalize(), since "finalize" has a pretty
> > specific meaning in GObject, and this function can be called from
> > more
> > places than the object's finalize stage. I wouldn't be opposed to
> > simply swapping the order of the words though. e.g.
> > streams_clear().
> > Or maybe streams_reset().
> >
> > In OO terms though, it is basically a member function of
> > SpiceDisplayChannel, and I tend to prefer using the full naming
> > convention, even for static functions (e.g.
> > spice_display_channel_streams_clear()?). But there's plenty of
> > precedent for shortening static functions in the code as well.
>
> I tend to prefer using full naming convention only for exported
> functions, for both internal and external usage.
>
> Not having the full name makes it clear it is a helper function in
> that
> context/file.
>
> Everyone has their preferences, I'm not picky about these things ;)
>
> Cheers,
Yeah, as I said, there's plenty of precedent in the codebase for
shortening helper functions, so I'm not really opposed. ;)
>
> >
> > Jonathon
> >
> >
> > >
> > > This function is called on:
> > > - display_handle_stream_destroy_all()
> > > - spice_display_channel_reset()
> > > - spice_display_channel_finalize()
> > >
> > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > > ---
> > > src/channel-display.c | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/src/channel-display.c b/src/channel-display.c
> > > index 9ae2851..a68a1ca 100644
> > > --- a/src/channel-display.c
> > > +++ b/src/channel-display.c
> > > @@ -104,7 +104,7 @@ static void
> > > spice_display_channel_up(SpiceChannel
> > > *channel);
> > > static void channel_set_handlers(SpiceChannelClass *klass);
> > >
> > > static void clear_surfaces(SpiceChannel *channel, gboolean
> > > keep_primary);
> > > -static void clear_streams(SpiceChannel *channel);
> > > +static void streams_finalize(SpiceChannel *channel);
> > > static void streams_check_init(SpiceChannel *channel, guint
> > > stream_id);
> > > static display_surface *find_surface(SpiceDisplayChannelPrivate
> > > *c,
> > > guint32 surface_id);
> > > static void spice_display_channel_reset(SpiceChannel *channel,
> > > gboolean migrating);
> > > @@ -172,7 +172,7 @@ static void
> > > spice_display_channel_finalize(GObject *object)
> > > g_clear_pointer(&c->monitors, g_array_unref);
> > > clear_surfaces(SPICE_CHANNEL(object), FALSE);
> > > g_hash_table_unref(c->surfaces);
> > > - clear_streams(SPICE_CHANNEL(object));
> > > + streams_finalize(SPICE_CHANNEL(object));
> > > g_clear_pointer(&c->palettes, cache_free);
> > >
> > > if (G_OBJECT_CLASS(spice_display_channel_parent_class)-
> > > > finalize)
> > >
> > > @@ -253,7 +253,7 @@ static void
> > > spice_display_set_property(GObject *object,
> > > static void spice_display_channel_reset(SpiceChannel *channel,
> > > gboolean migrating)
> > > {
> > > /* palettes, images, and glz_window are cleared in the
> > > session
> > > */
> > > - clear_streams(channel);
> > > + streams_finalize(channel);
> > > clear_surfaces(channel, TRUE);
> > >
> > > SPICE_CHANNEL_CLASS(spice_display_channel_parent_class)-
> > > > channel_reset(channel, migrating);
> > >
> > > @@ -1572,7 +1572,7 @@ static void
> > > destroy_display_stream(display_stream *st, int id)
> > > g_free(st);
> > > }
> > >
> > > -static void clear_streams(SpiceChannel *channel)
> > > +static void streams_finalize(SpiceChannel *channel)
> > > {
> > > SpiceDisplayChannelPrivate *c =
> > > SPICE_DISPLAY_CHANNEL(channel)-
> > > > priv;
> > >
> > > int i;
> > > @@ -1626,7 +1626,7 @@ static void
> > > display_handle_stream_destroy(SpiceChannel *channel, SpiceMsgIn
> > > *in)
> > > /* coroutine context */
> > > static void display_handle_stream_destroy_all(SpiceChannel
> > > *channel,
> > > SpiceMsgIn *in)
> > > {
> > > - clear_streams(channel);
> > > + streams_finalize(channel);
> > > }
> > >
> > > /* coroutine context */
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel