Re: [PATCH 02/10] Store QXLInstance in CursorItem

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

 



> 
> On Tue, Nov 3, 2015 at 6:00 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx>
> wrote:
> > On Mon, Nov 02, 2015 at 05:11:04PM +0100, Fabiano Fidêncio wrote:
> >> On Mon, Nov 2, 2015 at 5:00 PM, Frediano Ziglio <fziglio@xxxxxxxxxx>
> >> wrote:
> >> >>
> >> >> On Mon, Nov 2, 2015 at 10:55 AM, Frediano Ziglio <fziglio@xxxxxxxxxx>
> >> >> wrote:
> >> >> Just a nitpick, I would prefer to have a explicit comparison here: if
> >> >> (--items->refs > 0) ...
> >> >>
> >> >
> >> > Why not
> >> >
> >> >   if (--items->refs != 0) ??
> >> >
> >> > I mean, at the end the results should be the same if no errors managing
> >> > the counters are present.
> >>
> >> I just think the check for > 0 is the proper test we want to do,
> >> mainly considering that refs is a int (and not a uint)
> >>
> >
> > For what it's worth, we currently have all variants, with the 'no
> > explicit test' variant being much more common:
> >
> > $ git grep -- "--.*refs"
> > server/char_device.c:    if (--buf->refs == 0) {
> > server/char_device.c:        --buf->refs;
> > server/char_device.c:    if (!--char_dev->refs) {
> > server/cursor-channel.c:    if (--item->refs)
> > server/cursor-channel.c:    if (--pipe_item->refs) {
> > server/pixmap-cache.c:    if (--cache->refs) {
> > server/red_channel.c:    if (!--channel->refs) {
> > server/red_channel.c:    if (!--rcc->refs) {
> > server/red_channel.c:    if (!--client->refs) {
> > server/red_worker.c:    if (--monitors_config->refs > 0) {
> > server/red_worker.c:    if (--dpi->refs) {
> > server/red_worker.c:    if (!--item->refs) {
> > server/red_worker.c:    if (!--item->refs) {
> > server/red_worker.c:    if (!--surface->refs) {
> > server/red_worker.c:    if (--red_drawable->refs) {
> > server/red_worker.c:    if (!--drawable->refs) {
> > server/red_worker.c:    if (!--stream->refs) {
> > server/red_worker.c:    if (!--item->refs) {
> > server/red_worker.c:    if (--shared_dict->refs) {
> > server/reds.c:    if (!--buf->refs) {
> > server/smartcard.c:    if (!--item->refs) {
> > server/snd_worker.c:    if (!--channel->refs) {
> > server/spicevmc.c:    if (!--item->refs)
> >
> > I personally prefer explicit tests.
> 
> I also prefer explicit tests for everything that is not boolean.
> It's way easier to, on a quick look, understand what is that var that
> you never ever saw before ...
> 

All new reference checks follow this style.

Frediano

> > However, hopefully we'll manage to
> > get rid of much of this manual refcounting by the end of all this work,
> > so not really important.
> 
> Yeah, I agree!
> 
> >
> > Christophe
> 
_______________________________________________
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]