It's a more powerful version of qio_channel_add_watch(), which supports
non-default gcontext. It's stripped from the old one, then we have
g_source_get_id() to fetch the tag ID to keep the old interface.
Note that the new API will return a gsource, meanwhile keep a reference
of it so that callers need to unref them explicitly.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/io/channel.h | 31 ++++++++++++++++++++++++++++---
io/channel.c | 24 +++++++++++++++++++-----
2 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/include/io/channel.h b/include/io/channel.h
index 3995e243a3..36af5e58ae 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -620,20 +620,45 @@ GSource *qio_channel_create_watch(QIOChannel *ioc,
GIOCondition condition);
/**
- * qio_channel_add_watch:
+ * qio_channel_add_watch_full:
* @ioc: the channel object
* @condition: the I/O condition to monitor
* @func: callback to invoke when the source becomes ready
* @user_data: opaque data to pass to @func
* @notify: callback to free @user_data
+ * @context: gcontext to bind the source to
*
- * Create a new main loop source that is used to watch
+ * Create a new source that is used to watch
* for the I/O condition @condition. The callback @func
* will be registered against the source, to be invoked
* when the source becomes ready. The optional @user_data
* will be passed to @func when it is invoked. The @notify
* callback will be used to free @user_data when the
- * watch is deleted
+ * watch is deleted. The source will be bound to @context if
+ * provided, or main context if it is NULL.
+ *
+ * Note: if a valid source is returned, we need to explicitly unref
+ * the source to destroy it.
+ *
+ * Returns: the source pointer
+ */
+GSource *qio_channel_add_watch_full(QIOChannel *ioc,
+ GIOCondition condition,
+ QIOChannelFunc func,
+ gpointer user_data,
+ GDestroyNotify notify,
+ GMainContext *context);
+
+/**
+ * qio_channel_add_watch:
+ * @ioc: the channel object
+ * @condition: the I/O condition to monitor
+ * @func: callback to invoke when the source becomes ready
+ * @user_data: opaque data to pass to @func
+ * @notify: callback to free @user_data
+ *
+ * Wrapper of qio_channel_add_watch_full(), but it'll only bind the
+ * source object to default main context.
*
* The returned source ID can be used with g_source_remove()
* to remove and free the source when no longer required.
diff --git a/io/channel.c b/io/channel.c
index ec4b86de7c..3e734cc9a5 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -299,6 +299,22 @@ void qio_channel_set_aio_fd_handler(QIOChannel *ioc,
klass->io_set_aio_fd_handler(ioc, ctx, io_read, io_write, opaque);
}
+GSource *qio_channel_add_watch_full(QIOChannel *ioc,
+ GIOCondition condition,
+ QIOChannelFunc func,
+ gpointer user_data,
+ GDestroyNotify notify,
+ GMainContext *context)
+{
+ GSource *source;
+
+ source = qio_channel_create_watch(ioc, condition);
+ g_source_set_callback(source, (GSourceFunc)func, user_data, notify);
+ g_source_attach(source, context);
+
+ return source;
+}
+
guint qio_channel_add_watch(QIOChannel *ioc,
GIOCondition condition,
QIOChannelFunc func,
@@ -308,11 +324,9 @@ guint qio_channel_add_watch(QIOChannel *ioc,
GSource *source;
guint id;
- source = qio_channel_create_watch(ioc, condition);
-
- g_source_set_callback(source, (GSourceFunc)func, user_data, notify);
-
- id = g_source_attach(source, NULL);
+ source = qio_channel_add_watch_full(ioc, condition, func,
+ user_data, notify, NULL);
+ id = g_source_get_id(source);
g_source_unref(source);
return id;
--
2.14.3
On Wed, Feb 28, 2018 at 01:06:22PM +0800, Peter Xu wrote:
> It's a more powerful version of qio_channel_add_watch(), which supports
> non-default gcontext. It's stripped from the old one, then we have
> g_source_get_id() to fetch the tag ID to keep the old interface.
>
> Note that the new API will return a gsource, meanwhile keep a reference
> of it so that callers need to unref them explicitly.
I don't really like this. Having qio_channel_add_watch and
qio_channel_add_watch_full with differing return values is
really very surprising. They should be functionally identical,
except for the extra context arg.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> include/io/channel.h | 31 ++++++++++++++++++++++++++++---
> io/channel.c | 24 +++++++++++++++++++-----
> 2 files changed, 47 insertions(+), 8 deletions(-)
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index 3995e243a3..36af5e58ae 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -620,20 +620,45 @@ GSource *qio_channel_create_watch(QIOChannel *ioc,
> GIOCondition condition);
>
> /**
> - * qio_channel_add_watch:
> + * qio_channel_add_watch_full:
> * @ioc: the channel object
> * @condition: the I/O condition to monitor
> * @func: callback to invoke when the source becomes ready
> * @user_data: opaque data to pass to @func
> * @notify: callback to free @user_data
> + * @context: gcontext to bind the source to
> *
> - * Create a new main loop source that is used to watch
> + * Create a new source that is used to watch
> * for the I/O condition @condition. The callback @func
> * will be registered against the source, to be invoked
> * when the source becomes ready. The optional @user_data
> * will be passed to @func when it is invoked. The @notify
> * callback will be used to free @user_data when the
> - * watch is deleted
> + * watch is deleted. The source will be bound to @context if
> + * provided, or main context if it is NULL.
> + *
> + * Note: if a valid source is returned, we need to explicitly unref
> + * the source to destroy it.
> + *
> + * Returns: the source pointer
> + */
> +GSource *qio_channel_add_watch_full(QIOChannel *ioc,
> + GIOCondition condition,
> + QIOChannelFunc func,
> + gpointer user_data,
> + GDestroyNotify notify,
> + GMainContext *context);
> +
> +/**
> + * qio_channel_add_watch:
> + * @ioc: the channel object
> + * @condition: the I/O condition to monitor
> + * @func: callback to invoke when the source becomes ready
> + * @user_data: opaque data to pass to @func
> + * @notify: callback to free @user_data
> + *
> + * Wrapper of qio_channel_add_watch_full(), but it'll only bind the
> + * source object to default main context.
> *
> * The returned source ID can be used with g_source_remove()
> * to remove and free the source when no longer required.
> diff --git a/io/channel.c b/io/channel.c
> index ec4b86de7c..3e734cc9a5 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -299,6 +299,22 @@ void qio_channel_set_aio_fd_handler(QIOChannel *ioc,
> klass->io_set_aio_fd_handler(ioc, ctx, io_read, io_write, opaque);
> }
>
> +GSource *qio_channel_add_watch_full(QIOChannel *ioc,
> + GIOCondition condition,
> + QIOChannelFunc func,
> + gpointer user_data,
> + GDestroyNotify notify,
> + GMainContext *context)
> +{
> + GSource *source;
> +
> + source = qio_channel_create_watch(ioc, condition);
> + g_source_set_callback(source, (GSourceFunc)func, user_data, notify);
> + g_source_attach(source, context);
> +
> + return source;
> +}
> +
> guint qio_channel_add_watch(QIOChannel *ioc,
> GIOCondition condition,
> QIOChannelFunc func,
> @@ -308,11 +324,9 @@ guint qio_channel_add_watch(QIOChannel *ioc,
> GSource *source;
> guint id;
>
> - source = qio_channel_create_watch(ioc, condition);
> -
> - g_source_set_callback(source, (GSourceFunc)func, user_data, notify);
> -
> - id = g_source_attach(source, NULL);
> + source = qio_channel_add_watch_full(ioc, condition, func,
> + user_data, notify, NULL);
> + id = g_source_get_id(source);
> g_source_unref(source);
>
> return id;
> --
> 2.14.3
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Feb 28, 2018 at 09:08:45AM +0000, Daniel P. Berrangé wrote: > On Wed, Feb 28, 2018 at 01:06:22PM +0800, Peter Xu wrote: > > It's a more powerful version of qio_channel_add_watch(), which supports > > non-default gcontext. It's stripped from the old one, then we have > > g_source_get_id() to fetch the tag ID to keep the old interface. > > > > Note that the new API will return a gsource, meanwhile keep a reference > > of it so that callers need to unref them explicitly. > > I don't really like this. Having qio_channel_add_watch and > qio_channel_add_watch_full with differing return values is > really very surprising. They should be functionally identical, > except for the extra context arg. Yeah it's not nice, but I do need the GSource and the tag ID does not help in the series. An alternative would be that I modify qio_channel_add_watch() to return GSource too. Is there an third choice that you could suggest? Thanks, -- Peter Xu
On Wed, Feb 28, 2018 at 08:44:24PM +0800, Peter Xu wrote: > On Wed, Feb 28, 2018 at 09:08:45AM +0000, Daniel P. Berrangé wrote: > > On Wed, Feb 28, 2018 at 01:06:22PM +0800, Peter Xu wrote: > > > It's a more powerful version of qio_channel_add_watch(), which supports > > > non-default gcontext. It's stripped from the old one, then we have > > > g_source_get_id() to fetch the tag ID to keep the old interface. > > > > > > Note that the new API will return a gsource, meanwhile keep a reference > > > of it so that callers need to unref them explicitly. > > > > I don't really like this. Having qio_channel_add_watch and > > qio_channel_add_watch_full with differing return values is > > really very surprising. They should be functionally identical, > > except for the extra context arg. > > Yeah it's not nice, but I do need the GSource and the tag ID does not > help in the series. > > An alternative would be that I modify qio_channel_add_watch() to > return GSource too. Is there an third choice that you could suggest? Given you have the id + GMainContext you can just acquire the GSource, if needed, using g_main_context_find_source_by_id. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Feb 28, 2018 at 12:47:43PM +0000, Daniel P. Berrangé wrote: > On Wed, Feb 28, 2018 at 08:44:24PM +0800, Peter Xu wrote: > > On Wed, Feb 28, 2018 at 09:08:45AM +0000, Daniel P. Berrangé wrote: > > > On Wed, Feb 28, 2018 at 01:06:22PM +0800, Peter Xu wrote: > > > > It's a more powerful version of qio_channel_add_watch(), which supports > > > > non-default gcontext. It's stripped from the old one, then we have > > > > g_source_get_id() to fetch the tag ID to keep the old interface. > > > > > > > > Note that the new API will return a gsource, meanwhile keep a reference > > > > of it so that callers need to unref them explicitly. > > > > > > I don't really like this. Having qio_channel_add_watch and > > > qio_channel_add_watch_full with differing return values is > > > really very surprising. They should be functionally identical, > > > except for the extra context arg. > > > > Yeah it's not nice, but I do need the GSource and the tag ID does not > > help in the series. > > > > An alternative would be that I modify qio_channel_add_watch() to > > return GSource too. Is there an third choice that you could suggest? > > Given you have the id + GMainContext you can just acquire the GSource, > if needed, using g_main_context_find_source_by_id. I always feel unsafe to play around with tag IDs since the IDs can change after GSource removed and new GSource added, and also the result of the call will depend on a correct pairing of context (so if the context is incorrect, instead of failure, we possibly got everything screwed up while we never know we failed...). But indeed for this one it seems pretty safe if I call g_main_context_find_source_by_id() right after I call qio_channel_add_watch_full() to fetch the GSource. If you agree, I can use this approach in my next post. Thanks, -- Peter Xu
© 2016 - 2025 Red Hat, Inc.