linux-kernel - Re: [patch v9 3/4] platform/mellanox: mlxreg-hotplug: Code cleanup

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: <20180123035357.GD24956@fury>
Date:   Mon, 22 Jan 2018 19:53:57 -0800
From:   Darren Hart <dvhart@...radead.org>
To:     Vadim Pasternak <vadimp@...lanox.com>
Cc:     andy.shevchenko@...il.com, gregkh@...uxfoundation.org,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        jiri@...nulli.us
Subject: Re: [patch v9 3/4] platform/mellanox: mlxreg-hotplug: Code cleanup

On Wed, Jan 17, 2018 at 06:21:55PM +0000, Vadim Pasternak wrote:

As I continue to pick these patches apart into their individual functional
changes, I'm finding more issues...

> Renaming field bus in structure mlxreg_hotplug_device to nr to have a name
> consistent with Linux i2c naming.
> 
> Removing unnecessary includes.
> 
> Removing from mlxreg_hotplug_device_create dev_err to upper layer should
> produce error on EFAULT return.
> 

But the only caller doesn't check the return code. Where did you expect for this
error to be detected?

> Relocation mlxreg_hotplug_device_create/destroy to the beginning of the
> mlxreg-hotplug.c in order to have attribute related code at the same
> location.
> 
> Add to structure mlxreg_hotplug_device device node handle for devices
> defined in device table tree;
> 

The above should *really* be several changes. Start with the non-functional
preparatory changes, followed by single functional change. Also, it seems to me
adding the device node prior to enabling building on ARM would make sense??

I've broken this up, verified incremental build, and rewritten the commit
messages. I have this sitting in a local branch currently.

I need you to respond to the question above re the create call, but I'll make
the change myself.

-- 
Darren Hart
VMware Open Source Technology Center