linux-kernel - Re: [PATCH v3 1/3] firmware_class: 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]

Message-ID: <20160825175007.GA3296@wotan.suse.de>
Date:   Thu, 25 Aug 2016 19:50:07 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Daniel Wagner <wagi@...om.org>
Cc:     linux-kernel@...r.kernel.org,
        Daniel Wagner <daniel.wagner@...-carit.de>,
        Ming Lei <ming.lei@...onical.com>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH v3 1/3] firmware_class: encapsulate firmware loading
 status

On Thu, Aug 25, 2016 at 11:52:01AM +0200, Daniel Wagner wrote:
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 22d1760..f397026 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -91,10 +91,13 @@ static inline bool fw_is_builtin_firmware(const struct firmware *fw)
>  }
>  #endif
>  
> +#ifdef CONFIG_FW_LOADER_USER_HELPER
> +
>  enum {
> +	FW_STATUS_UNKNOWN,
>  	FW_STATUS_LOADING,
>  	FW_STATUS_DONE,
> -	FW_STATUS_ABORT,
> +	FW_STATUS_ABORTED,
>  };
>  
>  static int loading_timeout = 60;	/* In seconds */

<-- snip -->

> +#else /* CONFIG_FW_LOADER_USER_HELPER */
> +
> +static int loading_timeout = 60;
> +#define firmware_loading_timeout()		(loading_timeout * HZ)
> +
> +#define fw_status_wait_timeout(fw_st, long)	0

The timeout makes 0 sense for when !CONFIG_FW_LOADER_USER_HELPER so can
we do away with adding a silly 60 value to an int here and
the silly value of (loading_timeout * HZ) ? Its not used so its not
clear to me why this is here.

  Luis