[OpenWrt-Devel] [PATCH] libubus: handle NULL ubus_context in all sync requests by creating a local instance
Alexandru Ardelean
ardeleanalex at gmail.com
Thu Jul 17 11:27:37 EDT 2014
Forgot to add Github link for easier review.
https://github.com/commodo/ubus/commit/f8942190ea7ff4a243410aa0a9d38eacd41d0b4a
On Thu, Jul 17, 2014 at 6:25 PM, Alexandru Ardelean <ardeleanalex at gmail.com>
wrote:
> This can serve as a convenience for sync requets, since no explicit
> ubus_connect() + ubus_free() need to be done.
>
> It can also help in cases where there are async + sync requests,
> where one async request is working and sync calls are done.
> Seems to create some weird races, probably because of possible
> concurrent poll() calls on the same fd.
>
> ---
> libubus-obj.c | 42 +++++++++++++++++++++++++++++-------------
> libubus-req.c | 33 ++++++++++++++++++++++++++++-----
> libubus-sub.c | 15 +++++++++++++--
> libubus.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
> 4 files changed, 112 insertions(+), 26 deletions(-)
>
> diff --git a/libubus-obj.c b/libubus-obj.c
> index 47bdb0a..29fe538 100644
> --- a/libubus-obj.c
> +++ b/libubus-obj.c
> @@ -174,7 +174,13 @@ static bool ubus_push_object_type(const struct
> ubus_object_type *type)
> int ubus_add_object(struct ubus_context *ctx, struct ubus_object *obj)
> {
> struct ubus_request req;
> - int ret;
> + int ret = UBUS_STATUS_INVALID_ARGUMENT;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> blob_buf_init(&b, 0);
>
> @@ -184,22 +190,24 @@ int ubus_add_object(struct ubus_context *ctx, struct
> ubus_object *obj)
> if (obj->type->id)
> blob_put_int32(&b, UBUS_ATTR_OBJTYPE,
> obj->type->id);
> else if (!ubus_push_object_type(obj->type))
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
> }
>
> if (ubus_start_request(ctx, &req, b.head, UBUS_MSG_ADD_OBJECT, 0)
> < 0)
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
>
> req.raw_data_cb = ubus_add_object_cb;
> req.priv = obj;
> ret = ubus_complete_request(ctx, &req, 0);
> if (ret)
> - return ret;
> + goto out;
>
> - if (!obj->id)
> - return UBUS_STATUS_NO_DATA;
> + ret = (!obj->id) ? UBUS_STATUS_NO_DATA : 0;
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
>
> - return 0;
> + return ret;
> }
>
> static void ubus_remove_object_cb(struct ubus_request *req, int type,
> struct blob_attr *msg)
> @@ -221,22 +229,30 @@ static void ubus_remove_object_cb(struct
> ubus_request *req, int type, struct blo
> int ubus_remove_object(struct ubus_context *ctx, struct ubus_object *obj)
> {
> struct ubus_request req;
> - int ret;
> + int ret = UBUS_STATUS_INVALID_ARGUMENT;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> blob_buf_init(&b, 0);
> blob_put_int32(&b, UBUS_ATTR_OBJID, obj->id);
>
> if (ubus_start_request(ctx, &req, b.head, UBUS_MSG_REMOVE_OBJECT,
> 0) < 0)
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
>
> req.raw_data_cb = ubus_remove_object_cb;
> req.priv = obj;
> ret = ubus_complete_request(ctx, &req, 0);
> if (ret)
> - return ret;
> + goto out;
>
> - if (obj->id)
> - return UBUS_STATUS_NO_DATA;
> + ret = (!obj->id) ? UBUS_STATUS_NO_DATA : 0;
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
>
> - return 0;
> + return ret;
> }
> diff --git a/libubus-req.c b/libubus-req.c
> index 8475dc9..240c2d4 100644
> --- a/libubus-req.c
> +++ b/libubus-req.c
> @@ -225,14 +225,25 @@ int ubus_invoke(struct ubus_context *ctx, uint32_t
> obj, const char *method,
> {
> struct ubus_request req;
> int rc;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> rc = ubus_invoke_async(ctx, obj, method, msg, &req);
> if (rc)
> - return rc;
> + goto out;
>
> req.data_cb = cb;
> req.priv = priv;
> - return ubus_complete_request(ctx, &req, timeout);
> + rc = ubus_complete_request(ctx, &req, timeout);
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
> +
> + return rc;
> }
>
> static void
> @@ -288,17 +299,29 @@ int ubus_notify(struct ubus_context *ctx, struct
> ubus_object *obj,
> {
> struct ubus_notify_request req;
> int ret;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> ret = __ubus_notify_async(ctx, obj, type, msg, &req, timeout >= 0);
> if (ret < 0)
> - return ret;
> + goto out;
>
> if (timeout < 0) {
> ubus_abort_request(ctx, &req.req);
> - return 0;
> + ret = 0;
> + goto out;
> }
>
> - return ubus_complete_request(ctx, &req.req, timeout);
> + ret = ubus_complete_request(ctx, &req.req, timeout);
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
> +
> + return ret;
> }
>
> static bool ubus_get_status(struct ubus_msghdr *hdr, int *ret)
> diff --git a/libubus-sub.c b/libubus-sub.c
> index 8793133..1b51364 100644
> --- a/libubus-sub.c
> +++ b/libubus-sub.c
> @@ -45,16 +45,27 @@ static int
> __ubus_subscribe_request(struct ubus_context *ctx, struct ubus_object
> *obj, uint32_t id, int type)
> {
> struct ubus_request req;
> + int ret = UBUS_STATUS_INVALID_ARGUMENT;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> blob_buf_init(&b, 0);
> blob_put_int32(&b, UBUS_ATTR_OBJID, obj->id);
> blob_put_int32(&b, UBUS_ATTR_TARGET, id);
>
> if (ubus_start_request(ctx, &req, b.head, type, 0) < 0)
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
>
> - return ubus_complete_request(ctx, &req, 0);
> + ret = ubus_complete_request(ctx, &req, 0);
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
>
> + return ret;
> }
>
> int ubus_subscribe(struct ubus_context *ctx, struct ubus_subscriber *obj,
> uint32_t id)
> diff --git a/libubus.c b/libubus.c
> index be4e6ac..27e3d93 100644
> --- a/libubus.c
> +++ b/libubus.c
> @@ -156,18 +156,30 @@ int ubus_lookup(struct ubus_context *ctx, const char
> *path,
> ubus_lookup_handler_t cb, void *priv)
> {
> struct ubus_lookup_request lookup;
> + int ret = UBUS_STATUS_INVALID_ARGUMENT;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> blob_buf_init(&b, 0);
> if (path)
> blob_put_string(&b, UBUS_ATTR_OBJPATH, path);
>
> if (ubus_start_request(ctx, &lookup.req, b.head, UBUS_MSG_LOOKUP,
> 0) < 0)
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
>
> lookup.req.raw_data_cb = ubus_lookup_cb;
> lookup.req.priv = priv;
> lookup.cb = cb;
> - return ubus_complete_request(ctx, &lookup.req, 0);
> + ret = ubus_complete_request(ctx, &lookup.req, 0);
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
> +
> + return ret;
> }
>
> static void ubus_lookup_id_cb(struct ubus_request *req, int type, struct
> blob_attr *msg)
> @@ -186,18 +198,30 @@ static void ubus_lookup_id_cb(struct ubus_request
> *req, int type, struct blob_at
> int ubus_lookup_id(struct ubus_context *ctx, const char *path, uint32_t
> *id)
> {
> struct ubus_request req;
> + int ret = UBUS_STATUS_INVALID_ARGUMENT;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> blob_buf_init(&b, 0);
> if (path)
> blob_put_string(&b, UBUS_ATTR_OBJPATH, path);
>
> if (ubus_start_request(ctx, &req, b.head, UBUS_MSG_LOOKUP, 0) < 0)
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
>
> req.raw_data_cb = ubus_lookup_id_cb;
> req.priv = id;
>
> - return ubus_complete_request(ctx, &req, 0);
> + ret = ubus_complete_request(ctx, &req, 0);
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
> +
> + return ret;
> }
>
> static int ubus_event_cb(struct ubus_context *ctx, struct ubus_object
> *obj,
> @@ -252,6 +276,13 @@ int ubus_send_event(struct ubus_context *ctx, const
> char *id,
> {
> struct ubus_request req;
> void *s;
> + int ret = UBUS_STATUS_INVALID_ARGUMENT;
> + bool free_ctx = false;
> +
> + if (!ctx)
> + free_ctx = (ctx = ubus_connect(NULL)) != NULL;
> + if (!ctx)
> + return -1;
>
> blob_buf_init(&b, 0);
> blob_put_int32(&b, UBUS_ATTR_OBJID, UBUS_SYSTEM_OBJECT_EVENT);
> @@ -262,9 +293,14 @@ int ubus_send_event(struct ubus_context *ctx, const
> char *id,
> blob_nest_end(&b, s);
>
> if (ubus_start_request(ctx, &req, b.head, UBUS_MSG_INVOKE,
> UBUS_SYSTEM_OBJECT_EVENT) < 0)
> - return UBUS_STATUS_INVALID_ARGUMENT;
> + goto out;
> +
> + ret = ubus_complete_request(ctx, &req, 0);
> +out:
> + if (free_ctx)
> + ubus_free(ctx);
>
> - return ubus_complete_request(ctx, &req, 0);
> + return ret;
> }
>
> static void ubus_default_connection_lost(struct ubus_context *ctx)
> --
> 1.8.4.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20140717/fe7199ec/attachment.htm>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list