From: José Bollo <jose.bollo@iot.bzh>
Date: Fri, 13 Jul 2018 14:38:55 +0000 (+0200)
Subject: Simplify starting of services
X-Git-Tag: 5.99.2~6
X-Git-Url: https://gerrit.automotivelinux.org/gerrit/gitweb?p=src%2Fapp-framework-binder.git;a=commitdiff_plain;h=bebb8fa967f3b71b609119b293d2869d56036586

Simplify starting of services

The previous version was confusing and was expecting
that onneed and share_session were always true.

Removing this parameter simplifies the code.

Also handle errors of required classes and apis at initialisation.

Change-Id: I7c99aa356cba41f368bd47cab797fa086a5740af
Signed-off-by: José Bollo <jose.bollo@iot.bzh>
---

diff --git a/src/afb-api.h b/src/afb-api.h
index d03aaca5..821bccbe 100644
--- a/src/afb-api.h
+++ b/src/afb-api.h
@@ -24,7 +24,7 @@ struct json_object;
 struct afb_api_itf
 {
 	void (*call)(void *closure, struct afb_xreq *xreq);
-	int (*service_start)(void *closure, int share_session, int onneed);
+	int (*service_start)(void *closure);
 	void (*update_hooks)(void *closure);
 	int (*get_logmask)(void *closure);
 	void (*set_logmask)(void *closure, int level);
diff --git a/src/afb-apiset.c b/src/afb-apiset.c
index 0eb64a5a..994a9c7e 100644
--- a/src/afb-apiset.c
+++ b/src/afb-apiset.c
@@ -693,19 +693,18 @@ const struct afb_api_item *afb_apiset_lookup(struct afb_apiset *set, const char
 	return NULL;
 }
 
-static int start_api(struct api_desc *api, int share_session, int onneed);
+static int start_api(struct api_desc *api);
 
 /**
  * Start the apis of the 'array'
- * The attribute 'share_session' is sent to the start function.
  */
-static int start_array_apis(struct api_array *array, int share_session)
+static int start_array_apis(struct api_array *array)
 {
 	int i, rc = 0, rc2;
 
 	i = array->count;
 	while (i) {
-		rc2 = start_api(array->apis[--i], share_session, 1);
+		rc2 = start_api(array->apis[--i]);
 		if (rc2 < 0) {
 			rc = rc2;
 		}
@@ -715,24 +714,22 @@ static int start_array_apis(struct api_array *array, int share_session)
 
 /**
  * Start the class 'cla' (start the apis that provide it).
- * The attribute 'share_session' is sent to the start function.
  */
-static int start_class(struct api_class *cla, int share_session)
+static int start_class(struct api_class *cla)
 {
-	return start_array_apis(&cla->providers, share_session);
+	return start_array_apis(&cla->providers);
 }
 
 /**
  * Start the classes of the 'array'
- * The attribute 'share_session' is sent to the start function.
  */
-static int start_array_classes(struct api_array *array, int share_session)
+static int start_array_classes(struct api_array *array)
 {
 	int i, rc = 0, rc2;
 
 	i = array->count;
 	while (i) {
-		rc2 = start_class(array->classes[--i], share_session);
+		rc2 = start_class(array->classes[--i]);
 		if (rc2 < 0) {
 			rc = rc2;
 		}
@@ -742,9 +739,8 @@ static int start_array_classes(struct api_array *array, int share_session)
 
 /**
  * Start the depends of the 'array'
- * The attribute 'share_session' is sent to the start function.
  */
-static int start_array_depends(struct api_array *array, int share_session)
+static int start_array_depends(struct api_array *array)
 {
 	struct api_desc *api;
 	int i, rc = 0, rc2;
@@ -756,7 +752,7 @@ static int start_array_depends(struct api_array *array, int share_session)
 		if (!api)
 			rc = -1;
 		else {
-			rc2 = start_api(api, share_session, 1);
+			rc2 = start_api(api);
 			if (rc2 < 0) {
 				rc = rc2;
 			}
@@ -768,13 +764,9 @@ static int start_array_depends(struct api_array *array, int share_session)
 /**
  * Starts the service 'api'.
  * @param api the api
- * @param share_session if true start the servic"e in a shared session
- *                      if false start it in its own session
- * @param onneed if true start the service if possible, if false the api
- *               must be a service
  * @return a positive number on success
  */
-static int start_api(struct api_desc *api, int share_session, int onneed)
+static int start_api(struct api_desc *api)
 {
 	int rc;
 
@@ -786,20 +778,22 @@ static int start_api(struct api_desc *api, int share_session, int onneed)
 	}
 
 	INFO("API %s starting...", api->name);
-	rc = start_array_classes(&api->require.classes, share_session);
-	rc = start_array_depends(&api->require.apis, share_session);
-	if (api->api.itf->service_start) {
-		api->status = EBUSY;
-		rc = api->api.itf->service_start(api->api.closure, share_session, onneed);
-		if (rc < 0) {
-			api->status = errno ?: ECANCELED;
-			ERROR("The api %s failed to start (%d)", api->name, rc);
-			return -1;
+	api->status = EBUSY;
+	rc = start_array_classes(&api->require.classes);
+	if (rc < 0)
+		ERROR("Can start classes needed by api %s", api->name);
+	else {
+		rc = start_array_depends(&api->require.apis);
+		if (rc < 0)
+			ERROR("Can start apis needed by api %s", api->name);
+		else if (api->api.itf->service_start) {
+			rc = api->api.itf->service_start(api->api.closure);
+			if (rc < 0)
+				ERROR("The api %s failed to start", api->name);
 		}
-	} else if (!onneed) {
-		/* already started: it is an error */
-		ERROR("The api %s is not a startable service", api->name);
-		api->status = EINVAL;
+	}
+	if (rc < 0) {
+		api->status = errno ?: ECANCELED;
 		return -1;
 	}
 	NOTICE("API %s started", api->name);
@@ -820,7 +814,7 @@ const struct afb_api_item *afb_apiset_lookup_started(struct afb_apiset *set, con
 
 	i = lookup(set, name, rec);
 	if (i)
-		return i->status && start_api(i, 1, 1) ? NULL : &i->api;
+		return i->status && start_api(i) ? NULL : &i->api;
 	errno = ENOENT;
 	return NULL;
 }
@@ -829,13 +823,9 @@ const struct afb_api_item *afb_apiset_lookup_started(struct afb_apiset *set, con
  * Starts a service by its 'api' name.
  * @param set the api set
  * @param name name of the service to start
- * @param share_session if true start the servic"e in a shared session
- *                      if false start it in its own session
- * @param onneed if true start the service if possible, if false the api
- *               must be a service
  * @return a positive number on success
  */
-int afb_apiset_start_service(struct afb_apiset *set, const char *name, int share_session, int onneed)
+int afb_apiset_start_service(struct afb_apiset *set, const char *name)
 {
 	struct api_desc *a;
 
@@ -846,17 +836,15 @@ int afb_apiset_start_service(struct afb_apiset *set, const char *name, int share
 		return -1;
 	}
 
-	return start_api(a, share_session, onneed);
+	return start_api(a);
 }
 
 /**
  * Starts all possible services but stops at first error.
  * @param set the api set
- * @param share_session if true start the servic"e in a shared session
- *                      if false start it in its own session
  * @return 0 on success or a negative number when an error is found
  */
-int afb_apiset_start_all_services(struct afb_apiset *set, int share_session)
+int afb_apiset_start_all_services(struct afb_apiset *set)
 {
 	int rc, ret;
 	int i;
@@ -865,7 +853,7 @@ int afb_apiset_start_all_services(struct afb_apiset *set, int share_session)
 	while (set) {
 		i = 0;
 		while (i < set->apis.count) {
-			rc = start_api(set->apis.apis[i], share_session, 1);
+			rc = start_api(set->apis.apis[i]);
 			if (rc < 0)
 				ret = rc;
 			i++;
@@ -1120,11 +1108,10 @@ int afb_apiset_provide_class(struct afb_apiset *set, const char *apiname, const
 
 /**
  * Start any API that provides the class of name 'classname'
- * The attribute 'share_session' is sent to the start function.
  */
-int afb_apiset_class_start(const char *classname, int share_session)
+int afb_apiset_class_start(const char *classname)
 {
 	struct api_class *cla = class_search(classname, 0);
-	return cla ? start_class(cla, share_session) : (errno = ENOENT, -1);
+	return cla ? start_class(cla) : (errno = ENOENT, -1);
 }
 
diff --git a/src/afb-apiset.h b/src/afb-apiset.h
index fc8bc266..5aacecc6 100644
--- a/src/afb-apiset.h
+++ b/src/afb-apiset.h
@@ -51,8 +51,8 @@ extern const char *afb_apiset_unalias(struct afb_apiset *set, const char *name);
 extern const struct afb_api_item *afb_apiset_lookup(struct afb_apiset *set, const char *name, int rec);
 extern const struct afb_api_item *afb_apiset_lookup_started(struct afb_apiset *set, const char *name, int rec);
 
-extern int afb_apiset_start_service(struct afb_apiset *set, const char *name, int share_session, int onneed);
-extern int afb_apiset_start_all_services(struct afb_apiset *set, int share_session);
+extern int afb_apiset_start_service(struct afb_apiset *set, const char *name);
+extern int afb_apiset_start_all_services(struct afb_apiset *set);
 
 extern void afb_apiset_update_hooks(struct afb_apiset *set, const char *name);
 extern void afb_apiset_set_logmask(struct afb_apiset *set, const char *name, int mask);
@@ -70,4 +70,4 @@ extern void afb_apiset_enum(
 extern int afb_apiset_require(struct afb_apiset *set, const char *name, const char *required);
 extern int afb_apiset_require_class(struct afb_apiset *set, const char *apiname, const char *classname);
 extern int afb_apiset_provide_class(struct afb_apiset *set, const char *apiname, const char *classname);
-extern int afb_apiset_class_start(const char *classname, int share_session);
+extern int afb_apiset_class_start(const char *classname);
diff --git a/src/afb-export.c b/src/afb-export.c
index 315cc950..799ad73e 100644
--- a/src/afb-export.c
+++ b/src/afb-export.c
@@ -1552,29 +1552,23 @@ static void do_init(int sig, void *closure)
 };
 
 
-int afb_export_start(struct afb_export *export, int share_session, int onneed)
+int afb_export_start(struct afb_export *export)
 {
 	struct init init;
 	int rc;
 
 	/* check state */
-	if (export->state != Api_State_Pre_Init) {
-		/* not an error when onneed */
-		if (onneed != 0)
-			goto done;
+	switch (export->state) {
+	case Api_State_Run:
+		return 0;
 
-		/* already started: it is an error */
-		ERROR("Service of API %s already started", export->api.apiname);
+	case Api_State_Init:
+		/* starting in progress: it is an error */
+		ERROR("Service of API %s required started while starting", export->api.apiname);
 		return -1;
-	}
 
-	/* unshare the session if asked */
-	if (!share_session) {
-		rc = afb_export_unshare_session(export);
-		if (rc < 0) {
-			ERROR("Can't unshare the session for %s", export->api.apiname);
-			return -1;
-		}
+	default:
+		break;
 	}
 
 	/* set event handling */
@@ -1616,7 +1610,6 @@ int afb_export_start(struct afb_export *export, int share_session, int onneed)
 		return rc;
 	}
 
-done:
 	return 0;
 }
 
@@ -1668,11 +1661,11 @@ static struct json_object *api_describe_cb(void *closure)
 	return result;
 }
 
-static int api_service_start_cb(void *closure, int share_session, int onneed)
+static int api_service_start_cb(void *closure)
 {
 	struct afb_export *export = closure;
 
-	return afb_export_start(export, share_session, onneed);
+	return afb_export_start(export);
 }
 
 static void api_update_hooks_cb(void *closure)
diff --git a/src/afb-export.h b/src/afb-export.h
index c9046be8..be8adfd5 100644
--- a/src/afb-export.h
+++ b/src/afb-export.h
@@ -85,7 +85,7 @@ extern int afb_export_handle_init_v3(
 				struct afb_export *export,
 				int (*oninit)(struct afb_api_x3 *api));
 
-extern int afb_export_start(struct afb_export *export, int share_session, int onneed);
+extern int afb_export_start(struct afb_export *export);
 
 extern int afb_export_logmask_get(const struct afb_export *export);
 extern void afb_export_logmask_set(struct afb_export *export, int mask);
diff --git a/src/main-afb-daemon.c b/src/main-afb-daemon.c
index c1204464..ce681e38 100644
--- a/src/main-afb-daemon.c
+++ b/src/main-afb-daemon.c
@@ -619,7 +619,7 @@ static void start(int signum, void *arg)
 #if !defined(NO_CALL_PERSONALITY)
 	personality((unsigned long)-1L);
 #endif
-	if (afb_apiset_start_all_services(main_apiset, 1) < 0)
+	if (afb_apiset_start_all_services(main_apiset) < 0)
 		goto error;
 
 	/* start the HTTP server */
diff --git a/src/main-afs-supervisor.c b/src/main-afs-supervisor.c
index 14f1c3b0..461b0a0f 100644
--- a/src/main-afs-supervisor.c
+++ b/src/main-afs-supervisor.c
@@ -160,7 +160,7 @@ static void start(int signum, void *arg)
 	}
 
 	/* start the services */
-	if (afb_apiset_start_all_services(main_apiset, 1) < 0)
+	if (afb_apiset_start_all_services(main_apiset) < 0)
 		goto error;
 
 	/* start the HTTP server */
diff --git a/src/tests/apiset/test-apiset.c b/src/tests/apiset/test-apiset.c
index bdc84137..ccab4637 100644
--- a/src/tests/apiset/test-apiset.c
+++ b/src/tests/apiset/test-apiset.c
@@ -384,7 +384,7 @@ int set_cb_getmask(void *closure)
 	return set_last_api->mask;
 }
 
-int set_cb_start(void *closure, int share_session, int onneed)
+int set_cb_start(void *closure)
 {
 	set_cb0(closure);
 	ck_assert_int_eq(0, set_last_api->init);
@@ -422,7 +422,7 @@ START_TEST (check_settings)
 	nn = i;
 
 	set_count = 0;
-	afb_apiset_start_all_services(a, 1);
+	afb_apiset_start_all_services(a);
 	ck_assert_int_eq(nn, set_count);
 
 	set_count = 0;
@@ -480,7 +480,7 @@ struct clapi {
 
 int clorder;
 
-int clacb_start(void *closure, int share_session, int onneed)
+int clacb_start(void *closure)
 {
 	struct clapi *a = closure;
 	int i;
@@ -541,7 +541,7 @@ START_TEST (check_classes)
 	}
 
 	/* start all */
-	ck_assert_int_eq(0, afb_apiset_start_all_services(a, 0));
+	ck_assert_int_eq(0, afb_apiset_start_all_services(a));
 
 	afb_apiset_unref(a);
 }
diff --git a/src/tests/apiv3/test-apiv3.c b/src/tests/apiv3/test-apiv3.c
index dec1af97..9a06bcbc 100644
--- a/src/tests/apiv3/test-apiv3.c
+++ b/src/tests/apiv3/test-apiv3.c
@@ -165,7 +165,7 @@ START_TEST (test)
 	ck_assert_ptr_nonnull(out_api);
 
 	/* start all services */
-	rc = afb_apiset_start_all_services(apiset, 1);
+	rc = afb_apiset_start_all_services(apiset);
 	ck_assert_int_eq(rc, 0);
 }
 END_TEST