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 ...
> 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