linux-kernel - Re: [PATCH v5 2/5] firmware: encapsulate firmware loading status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 10 Sep 2016 00:24:46 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Daniel Wagner <wagi@...om.org>
Cc:     linux-kernel@...r.kernel.org, Ming Lei <ming.lei@...onical.com>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "rafael.j.wysocki" <rafael.j.wysocki@...el.com>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Takashi Iwai <tiwai@...e.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Arend van Spriel <arend.vanspriel@...adcom.com>,
        Daniel Wagner <daniel.wagner@...-carit.de>
Subject: Re: [PATCH v5 2/5] firmware: encapsulate firmware loading status

On Fri, Sep 09, 2016 at 02:12:21PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner@...-carit.de>
> 
> The firmware user helper code tracks the current state of the loading
> process via unsigned long status and a complection in struct
> firmware_buf. We only need this for the usermode helper as such we can
> encapsulate all this data into its own data structure.
> 
> Cc: Ming Lei <ming.lei@...onical.com>
> Cc: Luis R. Rodriguez <mcgrof@...nel.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Daniel Wagner <daniel.wagner@...-carit.de>
> ---
>  drivers/base/firmware_class.c | 130 +++++++++++++++++++++++++++++-------------
>  1 file changed, 91 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 8eba1fb..821babe 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -91,12 +91,6 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw)
>  }
>  #endif
>  
> -enum {
> -	FW_STATUS_LOADING,
> -	FW_STATUS_DONE,
> -	FW_STATUS_ABORT,
> -};
> -
>  static int loading_timeout = 60;	/* In seconds */
>  
>  static inline long firmware_loading_timeout(void)
> @@ -104,6 +98,77 @@ static inline long firmware_loading_timeout(void)
>  	return loading_timeout > 0 ? loading_timeout * HZ : MAX_JIFFY_OFFSET;
>  }
>  
> +#ifdef CONFIG_FW_LOADER_USER_HELPER
> +
> +enum {
> +	FW_UMH_UNKNOWN,
> +	FW_UMH_LOADING,
> +	FW_UMH_DONE,
> +	FW_UMH_ABORTED,
> +};

Note the enum here is anonymous still. That's a bit of a lost opportunity. If we name
it we can then just use it in arguments, but that can be done later in your other
patch. So say we name it enum fw_umh_status or whatever.

  Luis