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  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:   Tue, 22 Jan 2019 10:37:32 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Peng Hao <peng.hao2@....com.cn>
Cc:     arnd@...db.de, andy.shevchenko@...il.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6]  misc/pvpanic: Add pvpanic driver framework

On Tue, Jan 22, 2019 at 03:25:07AM +0800, Peng Hao wrote:
> Add pvpanic driver framework.
> 

You need a lot more description of what you did here than this, as I can
not understand from this text, what the patch does, or more importantly,
why it is doing this, at all.



> Signed-off-by: Peng Hao <peng.hao2@....com.cn>
> ---
>  drivers/misc/pvpanic/pvpanic.c | 171 ++++++++++-------------------------------
>  1 file changed, 39 insertions(+), 132 deletions(-)
> 
> diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
> index 595ac06..6380540 100644
> --- a/drivers/misc/pvpanic/pvpanic.c
> +++ b/drivers/misc/pvpanic/pvpanic.c
> @@ -8,15 +8,20 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/acpi.h>
> +#include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/types.h>
>  
> -static void __iomem *base;
> +static struct {
> +	struct platform_device *pdev;
> +	void __iomem *base;
> +	bool is_ioport;
> +} pvpanic_data = {
> +	.pdev = NULL,
> +	.is_ioport = false,

You do not need to initialize variables to 0 specifically like this.

> +};
>  
>  #define PVPANIC_PANICKED        (1 << 0)
>  
> @@ -27,7 +32,7 @@
>  static void
>  pvpanic_send_event(unsigned int event)
>  {
> -	iowrite8(event, base);
> +	iowrite8(event, pvpanic_data.base);

Why did you convert a single global variable into a single global
structure?  Why not, if you really need to pass this value around, do
that at the same time as you will end up touching these same functions
again, right?

thanks,

greg k-h

Powered by blists - more mailing lists