On Sun, Mar 26, 2017 at 6:21 PM, Alexandru Ardelean <ardeleana...@gmail.com> wrote: > On Sun, Mar 26, 2017 at 7:06 PM, Hans Dedecker <dedec...@gmail.com> wrote: >> On Mon, Mar 20, 2017 at 3:08 PM, Alexandru Ardelean >> <ardeleana...@gmail.com> wrote: >>> From: Alexandru Ardelean <ardeleana...@gmail.com> >>> >>> The context is that we generate some of the UCI config >>> for netifd via scripts/programs. >>> >>> Every once in a while, there's a goof when doing that >>> UCI generation, and netifd prints out the error at >>> stderr, but returns 0 (success) err-code. >>> >>> This change will fail the ubus call if UCI config >>> is invalid or missing for /etc/config/network. >>> >>> Signed-off-by: Alexandru Ardelean <ardeleana...@gmail.com> >>> --- >>> config.c | 10 ++++++++-- >>> config.h | 9 ++++++++- >>> main.c | 4 ++-- >>> netifd.h | 2 +- >>> ubus.c | 8 ++++++-- >>> 5 files changed, 25 insertions(+), 8 deletions(-) >>> >>> diff --git a/config.c b/config.c >>> index 0d965d3..3b1af82 100644 >>> --- a/config.c >>> +++ b/config.c >>> @@ -393,16 +393,20 @@ config_init_wireless(void) >>> vlist_flush(&wdev->interfaces); >>> } >>> >>> -void >>> +int >>> config_init_all(void) >>> { >>> + int ret = CONFIG_INIT_OK; >>> + >>> uci_network = config_init_package("network"); >>> if (!uci_network) { >>> fprintf(stderr, "Failed to load network config\n"); >>> - return; >>> + return CONFIG_INIT_ERR_NO_NETWORK; >>> } >>> >>> uci_wireless = config_init_package("wireless"); >>> + if (!uci_wireless) >>> + ret = CONFIG_INIT_ERR_NO_WIRELESS; >>> >>> vlist_update(&interfaces); >>> config_init = true; >>> @@ -426,4 +430,6 @@ config_init_all(void) >>> interface_refresh_assignments(false); >>> interface_start_pending(); >>> wireless_start_pending(); >>> + >>> + return ret; >>> } >>> diff --git a/config.h b/config.h >>> index 5adaca6..df30b64 100644 >>> --- a/config.h >>> +++ b/config.h >>> @@ -19,6 +19,13 @@ >>> >>> extern bool config_init; >>> >>> -void config_init_all(void); >>> +enum { >>> + CONFIG_INIT_OK, >>> + CONFIG_INIT_ERR_NO_WIRELESS, >>> + CONFIG_INIT_ERR_NO_NETWORK, >>> + __CONFIG_INIT_LAST >>> +}; >>> + >>> +int config_init_all(void); >>> >>> #endif >>> diff --git a/main.c b/main.c >>> index 5717b81..c173cef 100644 >>> --- a/main.c >>> +++ b/main.c >>> @@ -208,9 +208,9 @@ static void netifd_do_restart(struct uloop_timeout >>> *timeout) >>> execvp(global_argv[0], global_argv); >>> } >>> >>> -void netifd_reload(void) >>> +int netifd_reload(void) >>> { >>> - config_init_all(); >>> + return config_init_all(); >>> } >>> >>> void netifd_restart(void) >>> diff --git a/netifd.h b/netifd.h >>> index 5a90858..e565423 100644 >>> --- a/netifd.h >>> +++ b/netifd.h >>> @@ -98,6 +98,6 @@ struct interface; >>> extern const char *main_path; >>> extern const char *config_path; >>> void netifd_restart(void); >>> -void netifd_reload(void); >>> +int netifd_reload(void); >>> >>> #endif >>> diff --git a/ubus.c b/ubus.c >>> index 1b1a4cd..31e535c 100644 >>> --- a/ubus.c >>> +++ b/ubus.c >>> @@ -44,8 +44,12 @@ netifd_handle_reload(struct ubus_context *ctx, struct >>> ubus_object *obj, >>> struct ubus_request_data *req, const char *method, >>> struct blob_attr *msg) >>> { >>> - netifd_reload(); >>> - return 0; >>> + switch (netifd_reload()) { >>> + case CONFIG_INIT_ERR_NO_NETWORK: >>> + return UBUS_STATUS_UNKNOWN_ERROR; >> Is there any reason why only an ubus error code is returned for a >> network uci failure and not for a wireless uci failure ? >> >> Hans >>> + default: >>> + return UBUS_STATUS_OK; >>> + } >>> } >>> >>> enum { >>> -- >>> 2.7.4 >>> > > > That would make the "ubus call network reload" return non-zero. > Hmm, I guess that would not be a problem. > I interpreted Felix's comment a bit differently regarding not failing > the wireless config. I understood Felix's comment as still load the network config in case a wireless uci error is raised; but I would still return an error code.. This would make the CONFIG_INIT enum definition superfluous and will simplify the code.
Hans > > Alex _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev