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