Re: [PATCH 06/15] worker: minor simplifcation

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

 



On Tue, 2015-11-03 at 05:24 -0500, Frediano Ziglio wrote:
> I would change the comment to
> 
> "worker: remove unused worker parameter"
> 
Yes, it is more accurate

> > 
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > 
> > ---
> >  server/red_worker.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index 491e55e..fbcab6d 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -1506,16 +1506,18 @@ static inline Shadow *__find_shadow(TreeItem *item)
> >      return ((DrawItem *)item)->shadow;
> >  }
> >  
> > -static inline Ring *ring_of(RedWorker *worker, Ring *ring, TreeItem *item)
> > +/* FIXME: weird function: return container ring, or ring... */
> 
> I'd remove this comment.
> 
> > +static inline Ring *ring_of(Ring *ring, TreeItem *item)
> >  {
> >      return (item->container) ? &item->container->items : ring;
> >  }
> >  
> > -static inline int __contained_by(RedWorker *worker, TreeItem *item, Ring
> > *ring)
> > +/* FIXME: weird function: true if item has some parent container == ring,
> > or
> > has no immediate container*/
> 
> I'd remove this comment
> 
> > +static inline int __contained_by(TreeItem *item, Ring *ring)
> >  {
> >      spice_assert(item && ring);
> >      do {
> > -        Ring *now = ring_of(worker, ring, item);
> > +        Ring *now = ring_of(ring, item);
> >          if (now == ring) {
> >              return TRUE;
> >          }
> > @@ -1557,8 +1559,8 @@ static inline void __exclude_region(RedWorker *worker,
> > Ring *ring, TreeItem *ite
> >                      region_exclude(&shadow->on_hold, &and_rgn);
> >                      region_or(rgn, &and_rgn);
> >                      // in flat representation of current, shadow is always
> >                      his owner next
> > -                    if (!__contained_by(worker, (TreeItem*)shadow,
> > *top_ring)) {
> > -                        *top_ring = ring_of(worker, ring,
> > (TreeItem*)shadow);
> > +                    if (!__contained_by((TreeItem*)shadow, *top_ring)) {
> > +                        *top_ring = ring_of(ring, (TreeItem*)shadow);
> >                      }
> >                  }
> >              } else {
> > @@ -1577,8 +1579,8 @@ static inline void __exclude_region(RedWorker *worker,
> > Ring *ring, TreeItem *ite
> >                  region_exclude(rgn, &and_rgn);
> >                  if ((shadow = __find_shadow(item))) {
> >                      region_or(rgn, &shadow->on_hold);
> > -                    if (!__contained_by(worker, (TreeItem*)shadow,
> > *top_ring)) {
> > -                        *top_ring = ring_of(worker, ring,
> > (TreeItem*)shadow);
> > +                    if (!__contained_by((TreeItem*)shadow, *top_ring)) {
> > +                        *top_ring = ring_of(ring, (TreeItem*)shadow);
> >                      }
> >                  }
> >              }
> 
> If nobody complaints about these changes and if I get an ack before I'll
> change
> and merged this patch
> 
> Frediano

Ack the changes,
Pavel

> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://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]