[LEDE-DEV] [PATCH 1/3][RFC] netifd: propagate error code on netifd_reload()

[Alexandru Ardelean]

On Sun, Mar 26, 2017 at 7:06 PM, Hans Dedecker <dedeckeh at gmail.com> wrote:
> On Mon, Mar 20, 2017 at 3:08 PM, Alexandru Ardelean
> <ardeleanalex at gmail.com> wrote:
>> From: Alexandru Ardelean <ardeleanalex at 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 <ardeleanalex at 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.

Alex