From 44f21bd2a3b50f92669223cdafe79993654c1e19 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Jos=C3=A9=20Bollo?= <jose.bollo@iot.bzh>
Date: Mon, 27 Mar 2017 11:23:51 +0200
Subject: [PATCH] Simplify functions for calls
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

For historical reasons, the call to apis was
passing the length of the api and the length
of the verb. The reason was to avoid a copy
of strings. But the copy occured only for HTTP
requests. Having this implementation is of
small interest and compromise future changes.

This patch simplify things.

Change-Id: I8157724c6c721b6797cd0eab52b07e1b8d6eb5f8
Signed-off-by: José Bollo <jose.bollo@iot.bzh>
---
 src/afb-api-dbus.c | 16 +++++++++-------
 src/afb-api-so.c   |  6 +++---
 src/afb-api-ws.c   | 12 +++---------
 src/afb-apis.c     | 38 +++++++++++++++++++++++++++-----------
 src/afb-apis.h     |  5 ++---
 src/afb-hook.c     | 46 +++++++++++++++++++++++++++-------------------
 src/afb-hook.h     |  2 +-
 src/afb-hreq.c     | 16 ++++++++++++++++
 src/afb-hreq.h     |  4 ++++
 src/afb-hswitch.c  |  4 ++--
 src/afb-subcall.c  |  2 +-
 src/afb-svc.c      | 22 ++++++++++++----------
 src/afb-ws-json1.c |  2 +-
 13 files changed, 108 insertions(+), 67 deletions(-)

diff --git a/src/afb-api-dbus.c b/src/afb-api-dbus.c
index 4a5a4f1f..179f8954 100644
--- a/src/afb-api-dbus.c
+++ b/src/afb-api-dbus.c
@@ -317,11 +317,11 @@ static int api_dbus_client_on_reply(sd_bus_message *message, void *userdata, sd_
 }
 
 /* on call, propagate it to the dbus service */
-static void api_dbus_client_call(struct api_dbus *api, struct afb_req req, struct afb_context *context, const char *verb, size_t lenverb)
+static void api_dbus_client_call(void *closure, struct afb_req req, struct afb_context *context, const char *verb)
 {
+	struct api_dbus *api = closure;
 	size_t size;
 	int rc;
-	char *method = strndupa(verb, lenverb);
 	struct dbus_memo *memo;
 	struct sd_bus_message *msg;
 
@@ -334,7 +334,7 @@ static void api_dbus_client_call(struct api_dbus *api, struct afb_req req, struc
 
 	/* creates the message */
 	msg = NULL;
-	rc = sd_bus_message_new_method_call(api->sdbus, &msg, api->name, api->path, api->name, method);
+	rc = sd_bus_message_new_method_call(api->sdbus, &msg, api->name, api->path, api->name, verb);
 	if (rc < 0)
 		goto error;
 
@@ -363,8 +363,10 @@ end:
 	sd_bus_message_unref(msg);
 }
 
-static int api_dbus_service_start(struct api_dbus *api, int share_session, int onneed)
+static int api_dbus_service_start(void *closure, int share_session, int onneed)
 {
+	struct api_dbus *api = closure;
+
 	/* not an error when onneed */
 	if (onneed != 0)
 		return 0;
@@ -618,8 +620,8 @@ int afb_api_dbus_add_client(const char *path)
 
 	/* record it as an API */
 	afb_api.closure = api;
-	afb_api.call = (void*)api_dbus_client_call;
-	afb_api.service_start = (void*)api_dbus_service_start;
+	afb_api.call = api_dbus_client_call;
+	afb_api.service_start = api_dbus_service_start;
 	if (afb_apis_add(api->api, afb_api) < 0)
 		goto error2;
 
@@ -1003,7 +1005,7 @@ static int api_dbus_server_on_object_called(sd_bus_message *message, void *userd
 	dreq->refcount = 1;
 	areq.itf = &afb_api_dbus_req_itf;
 	areq.closure = dreq;
-	afb_apis_call_(areq, &dreq->context, api->api, method);
+	afb_apis_call(areq, &dreq->context, api->api, method);
 	dbus_req_unref(dreq);
 	return 1;
 
diff --git a/src/afb-api-so.c b/src/afb-api-so.c
index 222fbbbd..daa35334 100644
--- a/src/afb-api-so.c
+++ b/src/afb-api-so.c
@@ -182,16 +182,16 @@ static int call_check(struct afb_req req, struct afb_context *context, const str
 	return 1;
 }
 
-static void call_cb(void *closure, struct afb_req req, struct afb_context *context, const char *strverb, size_t lenverb)
+static void call_cb(void *closure, struct afb_req req, struct afb_context *context, const char *strverb)
 {
 	const struct afb_verb_desc_v1 *verb;
 	struct api_so_desc *desc = closure;
 
 	verb = desc->binding->v1.verbs;
-	while (verb->name && (strncasecmp(verb->name, strverb, lenverb) || verb->name[lenverb]))
+	while (verb->name && strcasecmp(verb->name, strverb))
 		verb++;
 	if (!verb->name)
-		afb_req_fail_f(req, "unknown-verb", "verb %.*s unknown within api %s", (int)lenverb, strverb, desc->binding->v1.prefix);
+		afb_req_fail_f(req, "unknown-verb", "verb %s unknown within api %s", strverb, desc->binding->v1.prefix);
 	else if (call_check(req, context, verb)) {
 		afb_thread_req_call(req, verb->callback, api_timeout, desc);
 	}
diff --git a/src/afb-api-ws.c b/src/afb-api-ws.c
index a56cfbef..89b83bc5 100644
--- a/src/afb-api-ws.c
+++ b/src/afb-api-ws.c
@@ -431,12 +431,6 @@ static int api_ws_write_uint32(struct writebuf *wb, uint32_t value)
 	return 1;
 }
 
-static int api_ws_write_string_nz(struct writebuf *wb, const char *value, size_t length)
-{
-	uint32_t len = (uint32_t)length;
-	return (size_t)len == length && ++len && api_ws_write_uint32(wb, len) && api_ws_write_put(wb, value, length) && api_ws_write_char(wb, '\0');
-}
-
 static int api_ws_write_string_length(struct writebuf *wb, const char *value, size_t length)
 {
 	uint32_t len = (uint32_t)++length;
@@ -815,7 +809,7 @@ static void api_ws_client_on_binary(void *closure, char *data, size_t size)
 }
 
 /* on call, propagate it to the ws service */
-static void api_ws_client_call_cb(void * closure, struct afb_req req, struct afb_context *context, const char *verb, size_t lenverb)
+static void api_ws_client_call_cb(void * closure, struct afb_req req, struct afb_context *context, const char *verb)
 {
 	int rc;
 	struct api_ws_memo *memo;
@@ -837,7 +831,7 @@ static void api_ws_client_call_cb(void * closure, struct afb_req req, struct afb
 		goto internal_error;
 	if (!api_ws_write_uint32(&wb, memo->msgid)
 	 || !api_ws_write_uint32(&wb, (uint32_t)context->flags)
-	 || !api_ws_write_string_nz(&wb, verb, lenverb)
+	 || !api_ws_write_string(&wb, verb)
 	 || !api_ws_write_string(&wb, afb_session_uuid(context->session))
 	 || !api_ws_write_string_length(&wb, raw, szraw))
 		goto overflow;
@@ -988,7 +982,7 @@ static void api_ws_server_called(struct api_ws_client *client, struct readbuf *r
 	/* makes the call */
 	areq.itf = &afb_api_ws_req_itf;
 	areq.closure = wreq;
-	afb_apis_call_(areq, &wreq->context, client->api, verb);
+	afb_apis_call(areq, &wreq->context, client->api, verb);
 	api_ws_server_req_unref(wreq);
 	return;
 
diff --git a/src/afb-apis.c b/src/afb-apis.c
index 7db3e75a..3463f535 100644
--- a/src/afb-apis.c
+++ b/src/afb-apis.c
@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <errno.h>
 
 #include "afb-session.h"
 #include "verbose.h"
@@ -32,24 +33,32 @@
 struct api_desc {
 	struct afb_api api;
 	const char *name;
-	size_t namelen;
 };
 
 static struct api_desc *apis_array = NULL;
 static int apis_count = 0;
 
+/**
+ * Returns the current count of APIs
+ */
 int afb_apis_count()
 {
 	return apis_count;
 }
 
+/**
+ * Checks wether 'name' is a valid API name.
+ * @return 1 if valid, 0 otherwise
+ */
 int afb_apis_is_valid_api_name(const char *name)
 {
 	unsigned char c;
 
 	c = (unsigned char)*name;
 	if (c == 0)
+		/* empty names aren't valid */
 		return 0;
+
 	do {
 		if (c < (unsigned char)'\x80') {
 			switch(c) {
@@ -74,6 +83,16 @@ int afb_apis_is_valid_api_name(const char *name)
 	return 1;
 }
 
+/**
+ * Adds the api of 'name' described by 'api'.
+ * @param name the name of the api to add
+ * @param api the api
+ * @returns 0 in case of success or -1 in case
+ * of error with errno set:
+ *   - EINVAL if name isn't valid
+ *   - EEXIST if name already registered
+ *   - ENOMEM when out of memory
+ */
 int afb_apis_add(const char *name, struct afb_api api)
 {
 	struct api_desc *apis;
@@ -82,6 +101,7 @@ int afb_apis_add(const char *name, struct afb_api api)
 	/* Checks the api name */
 	if (!afb_apis_is_valid_api_name(name)) {
 		ERROR("invalid api name forbidden (name is '%s')", name);
+		errno = EINVAL;
 		goto error;
 	}
 
@@ -89,6 +109,7 @@ int afb_apis_add(const char *name, struct afb_api api)
 	for (i = 0 ; i < apis_count ; i++) {
 		if (!strcasecmp(apis_array[i].name, name)) {
 			ERROR("api of name %s already exists", name);
+			errno = EEXIST;
 			goto error;
 		}
 	}
@@ -97,6 +118,7 @@ int afb_apis_add(const char *name, struct afb_api api)
 	apis = realloc(apis_array, ((unsigned)apis_count + 1) * sizeof * apis);
 	if (apis == NULL) {
 		ERROR("out of memory");
+		errno = ENOMEM;
 		goto error;
 	}
 	apis_array = apis;
@@ -104,7 +126,6 @@ int afb_apis_add(const char *name, struct afb_api api)
 	/* record the plugin */
 	apis = &apis_array[apis_count];
 	apis->api = api;
-	apis->namelen = strlen(name);
 	apis->name = name;
 	apis_count++;
 
@@ -114,22 +135,17 @@ error:
 	return -1;
 }
 
-void afb_apis_call_(struct afb_req req, struct afb_context *context, const char *api, const char *verb)
-{
-	afb_apis_call(req, context, api, strlen(api), verb, strlen(verb));
-}
-
-void afb_apis_call(struct afb_req req, struct afb_context *context, const char *api, size_t lenapi, const char *verb, size_t lenverb)
+void afb_apis_call(struct afb_req req, struct afb_context *context, const char *api, const char *verb)
 {
 	int i;
 	const struct api_desc *a;
 
-	req = afb_hook_req_call(req, context, api, lenapi, verb, lenverb);
+	req = afb_hook_req_call(req, context, api, verb);
 	a = apis_array;
 	for (i = 0 ; i < apis_count ; i++, a++) {
-		if (a->namelen == lenapi && !strncasecmp(a->name, api, lenapi)) {
+		if (!strcasecmp(a->name, api)) {
 			context->api_index = i;
-			a->api.call(a->api.closure, req, context, verb, lenverb);
+			a->api.call(a->api.closure, req, context, verb);
 			return;
 		}
 	}
diff --git a/src/afb-apis.h b/src/afb-apis.h
index 1dd9566b..3e90f661 100644
--- a/src/afb-apis.h
+++ b/src/afb-apis.h
@@ -23,7 +23,7 @@ struct afb_context;
 struct afb_api
 {
 	void *closure;
-	void (*call)(void *closure, struct afb_req req, struct afb_context *context, const char *verb, size_t lenverb);
+	void (*call)(void *closure, struct afb_req req, struct afb_context *context, const char *verb);
 	int (*service_start)(void *closure, int share_session, int onneed);
 };
 
@@ -36,7 +36,6 @@ extern int afb_apis_add(const char *name, struct afb_api api);
 extern int afb_apis_start_all_services(int share_session);
 extern int afb_apis_start_service(const char *name, int share_session, int onneed);
 
-extern void afb_apis_call(struct afb_req req, struct afb_context *context, const char *api, size_t lenapi, const char *verb, size_t lenverb);
-extern void afb_apis_call_(struct afb_req req, struct afb_context *context, const char *api, const char *verb);
+extern void afb_apis_call(struct afb_req req, struct afb_context *context, const char *api, const char *verb);
 
 
diff --git a/src/afb-hook.c b/src/afb-hook.c
index d5c534d0..f59192a7 100644
--- a/src/afb-hook.c
+++ b/src/afb-hook.c
@@ -272,25 +272,33 @@ static void hook_req_unref(struct afb_hook_req *tr)
 	}
 }
 
-static struct afb_hook_req *hook_req_create(struct afb_req req, struct afb_context *context, const char *api, size_t lenapi, const char *verb, size_t lenverb)
+static struct afb_hook_req *hook_req_create(struct afb_req req, struct afb_context *context, const char *api, const char *verb)
 {
+	int len;
+	char name[257];
 	unsigned id;
 	struct afb_hook_req *tr;
 
-	tr = malloc(sizeof *tr + 8 + lenapi + lenverb);
-	if (tr != NULL) {
-		/* get the call id */
-		id = ++hook_count;
-		if (id == 1000000)
-			id = hook_count = 1;
-
-		/* init hook */
-		tr->observers = NULL;
-		tr->refcount = 1;
-		tr->context = context;
-		tr->req = req;
-		afb_req_addref(req);
-		snprintf(tr->name, 9 + lenapi + lenverb, "%06d:%.*s/%.*s", id, (int)lenapi, api, (int)lenverb, verb);
+	/* get the call id */
+	id = ++hook_count;
+	if (id == 1000000)
+		id = hook_count = 1;
+
+	/* creates the name */
+	len = snprintf(name, sizeof name, "%06d:%s/%s", id, api, verb);
+	if (len < 0 || (size_t)len >= sizeof name) {
+		tr = NULL;
+	} else {
+		tr = malloc(sizeof *tr + (size_t)len);
+		if (tr != NULL) {
+			/* init hook */
+			tr->observers = NULL;
+			tr->refcount = 1;
+			tr->context = context;
+			tr->req = req;
+			afb_req_addref(req);
+			memcpy(tr->name, name, (size_t)(len + 1));
+		}
 	}
 	return tr;
 }
@@ -494,7 +502,7 @@ static struct afb_req_itf req_hook_itf = {
  * section: 
  *****************************************************************************/
 
-struct afb_req afb_hook_req_call(struct afb_req req, struct afb_context *context, const char *api, size_t lenapi, const char *verb, size_t lenverb)
+struct afb_req afb_hook_req_call(struct afb_req req, struct afb_context *context, const char *api, const char *verb)
 {
 	int add;
 	struct afb_hook_req *tr;
@@ -506,11 +514,11 @@ struct afb_req afb_hook_req_call(struct afb_req req, struct afb_context *context
 		do {
 			add = (hook->flags & afb_hook_flags_req_all) != 0
 			   && (!hook->session || hook->session == context->session)
-			   && (!hook->api || !(memcmp(hook->api, api, lenapi) || hook->api[lenapi]))
-			   && (!hook->verb || !(memcmp(hook->verb, verb, lenverb) || hook->verb[lenverb]));
+			   && (!hook->api || !strcasecmp(hook->api, api))
+			   && (!hook->verb || !strcasecmp(hook->verb, verb));
 			if (add) {
 				if (!tr)
-					tr = hook_req_create(req, context, api, lenapi, verb, lenverb);
+					tr = hook_req_create(req, context, api, verb);
 				if (tr)
 					hook_req_add_observer(tr, hook);
 			}
diff --git a/src/afb-hook.h b/src/afb-hook.h
index 2ad01b85..3edf8102 100644
--- a/src/afb-hook.h
+++ b/src/afb-hook.h
@@ -88,7 +88,7 @@ struct afb_hook_req_itf {
 	void (*hook_req_subcall_result)(void * closure, const struct afb_hook_req *tr, int status, struct json_object *result);
 };
 
-extern struct afb_req afb_hook_req_call(struct afb_req req, struct afb_context *context, const char *api, size_t lenapi, const char *verb, size_t lenverb);
+extern struct afb_req afb_hook_req_call(struct afb_req req, struct afb_context *context, const char *api, const char *verb);
 
 extern struct afb_hook *afb_hook_req_create(const char *api, const char *verb, struct afb_session *session, unsigned flags, struct afb_hook_req_itf *itf, void *closure);
 extern struct afb_hook *afb_hook_addref(struct afb_hook *spec);
diff --git a/src/afb-hreq.c b/src/afb-hreq.c
index 00d4b0eb..0fe908cb 100644
--- a/src/afb-hreq.c
+++ b/src/afb-hreq.c
@@ -335,6 +335,8 @@ void afb_hreq_unref(struct afb_hreq *hreq)
 	}
 	afb_context_disconnect(&hreq->context);
 	json_object_put(hreq->json);
+	free(hreq->api);
+	free(hreq->verb);
 	free(hreq);
 }
 
@@ -940,6 +942,20 @@ static void req_subcall(struct afb_hreq *hreq, const char *api, const char *verb
 	afb_subcall(&hreq->context, api, verb, args, callback, closure, (struct afb_req){ .itf = &afb_hreq_req_itf, .closure = hreq });
 }
 
+int afb_hreq_init_req_call(struct afb_hreq *hreq, const char *api, size_t lenapi, const char *verb, size_t lenverb)
+{
+	free(hreq->api);
+	free(hreq->verb);
+	hreq->api = strndup(api, lenapi);
+	hreq->verb = strndup(verb, lenverb);
+	if (hreq->api == NULL || hreq->verb == NULL) {
+		ERROR("Out of memory");
+		errno = ENOMEM;
+		return -1;
+	}
+	return afb_hreq_init_context(hreq);
+}
+
 int afb_hreq_init_context(struct afb_hreq *hreq)
 {
 	const char *uuid;
diff --git a/src/afb-hreq.h b/src/afb-hreq.h
index f07f2fa6..5cb86091 100644
--- a/src/afb-hreq.h
+++ b/src/afb-hreq.h
@@ -51,6 +51,8 @@ struct afb_hreq {
 	struct hreq_data *data;
 	struct json_object *json;
 	int upgrade;
+	char *api;
+	char *verb;
 };
 
 extern int afb_hreq_unprefix(struct afb_hreq *request, const char *prefix, size_t length);
@@ -83,6 +85,8 @@ extern int afb_hreq_post_add(struct afb_hreq *hreq, const char *name, const char
 
 extern struct afb_req afb_hreq_to_req(struct afb_hreq *hreq);
 
+extern int afb_hreq_init_req_call(struct afb_hreq *hreq, const char *api, size_t lenapi, const char *verb, size_t lenverb);
+
 extern int afb_hreq_init_context(struct afb_hreq *hreq);
 
 extern int afb_hreq_init_cookie(int port, const char *path, int maxage);
diff --git a/src/afb-hswitch.c b/src/afb-hswitch.c
index 9c929e83..909480af 100644
--- a/src/afb-hswitch.c
+++ b/src/afb-hswitch.c
@@ -44,10 +44,10 @@ int afb_hswitch_apis(struct afb_hreq *hreq, void *data)
 	if (!(*api && *verb && lenapi && lenverb))
 		return 0;
 
-	if (afb_hreq_init_context(hreq) < 0)
+	if (afb_hreq_init_req_call(hreq, api, lenapi, verb, lenverb) < 0)
 		afb_hreq_reply_error(hreq, MHD_HTTP_INTERNAL_SERVER_ERROR);
 	else
-		afb_apis_call(afb_hreq_to_req(hreq), &hreq->context, api, lenapi, verb, lenverb);
+		afb_apis_call(afb_hreq_to_req(hreq), &hreq->context, hreq->api, hreq->verb);
 	return 1;
 }
 
diff --git a/src/afb-subcall.c b/src/afb-subcall.c
index 00547c47..65bef2ff 100644
--- a/src/afb-subcall.c
+++ b/src/afb-subcall.c
@@ -187,7 +187,7 @@ void afb_subcall(struct afb_context *context, const char *api, const char *verb,
 	subcall->closure = closure;
 	subcall->context = *context;
 	afb_req_addref(req);
-	afb_apis_call_((struct afb_req){ .itf = &afb_subcall_req_itf, .closure = subcall }, &subcall->context, api, verb);
+	afb_apis_call((struct afb_req){ .itf = &afb_subcall_req_itf, .closure = subcall }, &subcall->context, api, verb);
 	subcall_unref(subcall);
 }
 
diff --git a/src/afb-svc.c b/src/afb-svc.c
index c52ad654..20dcdcfc 100644
--- a/src/afb-svc.c
+++ b/src/afb-svc.c
@@ -64,19 +64,19 @@ struct svc_req
 };
 
 /* functions for services */
-static void svc_on_event(struct afb_svc *svc, const char *event, int eventid, struct json_object *object);
-static void svc_call(struct afb_svc *svc, const char *api, const char *verb, struct json_object *args,
-				void (*callback)(void*, int, struct json_object*), void *closure);
+static void svc_on_event(void *closure, const char *event, int eventid, struct json_object *object);
+static void svc_call(void *closure, const char *api, const char *verb, struct json_object *args,
+				void (*callback)(void*, int, struct json_object*), void *cbclosure);
 
 /* the interface for services */
 static const struct afb_service_itf service_itf = {
-	.call = (void*)svc_call
+	.call = svc_call
 };
 
 /* the interface for events */
 static const struct afb_evt_itf evt_itf = {
-	.broadcast = (void*)svc_on_event,
-	.push = (void*)svc_on_event
+	.broadcast = svc_on_event,
+	.push = svc_on_event
 };
 
 /* functions for requests of services */
@@ -163,8 +163,9 @@ error:
 /*
  * Propagates the event to the service
  */
-static void svc_on_event(struct afb_svc *svc, const char *event, int eventid, struct json_object *object)
+static void svc_on_event(void *closure, const char *event, int eventid, struct json_object *object)
 {
+	struct afb_svc *svc = closure;
 	svc->on_event(event, object);
 	json_object_put(object);
 }
@@ -172,14 +173,15 @@ static void svc_on_event(struct afb_svc *svc, const char *event, int eventid, st
 /*
  * Initiates a call for the service
  */
-static void svc_call(struct afb_svc *svc, const char *api, const char *verb, struct json_object *args, void (*callback)(void*, int, struct json_object*), void *closure)
+static void svc_call(void *closure, const char *api, const char *verb, struct json_object *args, void (*callback)(void*, int, struct json_object*), void *cbclosure)
 {
+	struct afb_svc *svc = closure;
 	struct svc_req *svcreq;
 
 	/* allocates the request */
 	svcreq = malloc(sizeof *svcreq);
 	if (svcreq == NULL)
-		return afb_subcall_internal_error(callback, closure);
+		return afb_subcall_internal_error(callback, cbclosure);
 
 	/* initialises the request */
 	afb_context_init(&svcreq->context, svc->session, NULL);
@@ -188,7 +190,7 @@ static void svc_call(struct afb_svc *svc, const char *api, const char *verb, str
 	svcreq->refcount = 1;
 
 	/* makes the call */
-	afb_subcall(&svcreq->context, api, verb, args, callback, closure, (struct afb_req){ .itf = &afb_svc_req_itf, .closure = svcreq });
+	afb_subcall(&svcreq->context, api, verb, args, callback, cbclosure, (struct afb_req){ .itf = &afb_svc_req_itf, .closure = svcreq });
 
 	/* terminates and frees ressources if needed */
 	svcreq_unref(svcreq);
diff --git a/src/afb-ws-json1.c b/src/afb-ws-json1.c
index f383868a..8309065d 100644
--- a/src/afb-ws-json1.c
+++ b/src/afb-ws-json1.c
@@ -220,7 +220,7 @@ static void aws_on_call(struct afb_ws_json1 *ws, const char *api, const char *ve
 	/* emits the call */
 	r.closure = wsreq;
 	r.itf = &afb_ws_json1_req_itf;
-	afb_apis_call_(r, &wsreq->context, api, verb);
+	afb_apis_call(r, &wsreq->context, api, verb);
 	wsreq_unref(wsreq);
 }
 
-- 
2.16.6