[Spice-devel] bool or gboolean

[Frediano Ziglio]

> 
> On Wed, 2017-02-15 at 18:01 +0100, Christophe Fergeau wrote:
> > On Wed, Feb 15, 2017 at 11:39:32AM -0500, Frediano Ziglio wrote:
> > > Hi,
> > >   question was raised recently on the ML and IRC.
> > > 
> > > Some time ago we decided to use gboolean but some of us would like
> > > to discuss again.
> > 
> > So... As said a few times, I would have preferred to wait for a bit
> > to
> > have that discussion as I have a concrete patch which it would go
> > well
> > with...
> > 
> > This started with Jonathon's comment in
> > https://lists.freedesktop.org/archives/spice-devel/2017-February/0355
> > 79.html
> > where I returned -1 from a function which should be returning a
> > boolean.
> > The function was declared as 'int foo()', so it was not clear from
> > its
> > prototype that it's returning a bool, and I had totally missed that.
> > So I decided to go ahead and grep'ed for 'return (TRUE|FALSE)' over
> > the
> > code base, and make sure all these functions don't return int.
> > 
> > 
> > I started with 'gboolean', but then realized that using 'bool', the
> > compiler would be able to help me.
> > 
> > static int foo(void);
> > 
> > static gboolean foo(void)
> > { }
> > 
> > does not raise warnings while
> > 
> > static int foo(void);
> > 
> > static bool foo(void)
> > { }
> > 
> > does raise a warning, and it's the same with function pointers (see
> > attached bool.c file for a test case)
> > 
> > 
> > So after realizing that, I'd favour bool over gboolean, at least in
> > function prototypes, as it gives us slightly stronger typing.
> 
> That's a pretty strong argument for bool, in my opinion. In general,
> I'm in favor of increased type-safety. Daniel's point about glib
> requiring gboolean for callbacks etc is valid, but I'm curious about
> how many cases there would be. It looks like the patch that you
> attached only attempts to change int types to bool, but doesn't touch
> any existing gboolean types, right? It would be interesting to try to
> convert gboolean to bool to see how many of them can be easily changed.
> 

I did the check

./event-loop.c:static gboolean timer_func(gpointer user_data)
./tests/replay.c:static gboolean print_count = FALSE;
./tests/replay.c:static gboolean fill_queue_idle(gpointer user_data)
./tests/replay.c:static gboolean progress_timer(gpointer user_data)
./tests/replay.c:    gboolean wait = FALSE;
./tests/test-gst.c:static gboolean top_down = FALSE;
./tests/test-gst.c:    gboolean use_hw_encoder = FALSE; // TODO use
./red-worker.c:static gboolean worker_source_prepare(GSource *source, gint *p_timeout)
./red-worker.c:static gboolean worker_source_check(GSource *source)
./red-worker.c:static gboolean worker_source_dispatch(GSource *source, GSourceFunc callback,
./red-channel-client.c:static gboolean red_channel_client_initable_init(GInitable *initable,
./red-channel-client.c:static gboolean red_channel_client_initable_init(GInitable *initable,

> 
> > 
> > Regarding TRUE vs true, I don't really care, I'd stick with TRUE as
> > that
> > means less churn on the codebase, but I agree it's not really nice.
> 
> After you told me that trick with tig blame, I'm not quite as concerned
> about code churn as I used to be ;) A third option would be to
> standardize on 'true/false' for new code and slowly change existing
> TRUE/FALSE as we modify code...
> 
> Jonathon
> 

Frediano