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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190724093357.GA29532@kroah.com>
Date:   Wed, 24 Jul 2019 11:33:57 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Wu Hao <hao.wu@...el.com>
Cc:     mdf@...nel.org, linux-fpga@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
        linux-doc@...r.kernel.org, atull@...nel.org,
        Zhang Yi Z <yi.z.zhang@...el.com>,
        Xu Yilun <yilun.xu@...el.com>
Subject: Re: [PATCH v3 02/12] fpga: dfl: fme: add
 DFL_FPGA_FME_PORT_RELEASE/ASSIGN ioctl support.

On Tue, Jul 23, 2019 at 12:51:25PM +0800, Wu Hao wrote:
> +/**
> + * dfl_fpga_cdev_config_port - configure a port feature dev
> + * @cdev: parent container device.
> + * @port_id: id of the port feature device.
> + * @release: release port or assign port back.
> + *
> + * This function allows user to release port platform device or assign it back.
> + * e.g. to safely turn one port from PF into VF for PCI device SRIOV support,
> + * release port platform device is one necessary step.
> + */
> +int dfl_fpga_cdev_config_port(struct dfl_fpga_cdev *cdev, int port_id,
> +			      bool release)
> +{
> +	return release ? detach_port_dev(cdev, port_id) :
> +			 attach_port_dev(cdev, port_id);
> +}
> +EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_port);

That's a horrible api.  Every time you see this call in code, you have
to go and look up what "bool" means here.  There's no reason for it.

Just have 2 different functions, one that attaches a port, and one that
detaches it.  That way when you read the code that calls this function,
you know what it does instantly without having to go look up some api
function somewhere else.

Write code for people to read first.  And you are saving nothing here by
trying to do two different things in the same exact function.

thanks,

greg k-h

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ