linux-kernel - Re: [PATCH v5 08/11] scsi: host template attribute groups

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:   Tue, 6 Feb 2018 19:06:12 +0000
From:   Bart Van Assche <Bart.VanAssche@....com>
To:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jaegeuk@...nel.org" <jaegeuk@...nel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Stanislav Nijnikov <Stanislav.Nijnikov@....com>
CC:     Alex Lemberg <Alex.Lemberg@....com>
Subject: Re: [PATCH v5 08/11] scsi: host template attribute groups

On Tue, 2018-02-06 at 18:06 +0200, Stanislav Nijnikov wrote:
> +	if (sdev->host->hostt->sdev_groups) {
> +		error = sysfs_create_groups(&sdev->sdev_gendev.kobj,
> +			sdev->host->hostt->sdev_groups);
> +			if (error)
> +				return error;
> +	}

Please follow the indentation style that is used elsewhere in the kernel and
move the sdev->host->hostt->sdev_groups argument to the right such that it
starts one position to the right of the sysfs_create_groups() opening
parenthesis. Please also indent the if (error) statement correctly.

> @@ -1326,6 +1333,11 @@ void __scsi_remove_device(struct scsi_device *sdev)
>  	if (sdev->sdev_state == SDEV_DEL)
>  		return;
>  
> +	if (sdev->host->hostt->sdev_groups) {
> +		sysfs_remove_groups(&sdev->sdev_gendev.kobj,
> +			sdev->host->hostt->sdev_groups);
> +	}

If sdev->is_visible is false then the device_add() call for sdev->sdev_gendev
may have failed so calling sysfs_remove_groups() is not necessary. Please move
this call down to just before the device_del(dev) since that call also removes
sysfs attributes. Additionally, I think that checkpatch will complain about the
above code and will report that braces are not necessary for single-line
statements?

Thanks,

Bart.