[systemd-devel] [PATCH] udev: build by-path identifiers for ATA

[David Milburn]

Hi Lennart,

On 09/06/2015 05:48 AM, Lennart Poettering wrote:
> On Fri, 04.09.15 10:59, David Milburn (dmilburn at redhat.com) wrote:
>
> Please file this issue as github PR:
>
> https://github.com/systemd/systemd

Ok, did that.

>
>>
>> +static struct udev_device *handle_scsi_ata(struct udev_device *parent, char **path)
>> +{
>
> Please reading CODING_STYLE. We tend to place the opening bracket on
> the same line as the function name.
>
>> +	struct udev *udev  = udev_device_get_udev(parent);
>> +	struct udev_device *targetdev;
>> +	struct udev_device *target_parent;
>> +	struct udev_device *atadev;
>> +	const char *port_no;
>> +
>> +	targetdev = udev_device_get_parent_with_subsystem_devtype(parent, "scsi", "scsi_host");
>> +	if (targetdev == NULL)
>> +		return NULL;
>
> Usually, we don' compare explicitly with NULL.

Ok, fixed that.

>
>> +
>> +	target_parent = udev_device_get_parent(targetdev);
>> +	if (target_parent == NULL)
>> +		return NULL;
>> +
>> +	atadev = udev_device_new_from_subsystem_sysname(udev, "ata_port",
>> +							udev_device_get_sysname(target_parent));
>> +	if (atadev == NULL)
>> +		return NULL;
>> +
>> +	port_no = udev_device_get_sysattr_value(atadev, "port_no");
>> +	if (port_no == NULL) {
>> +		parent = NULL;
>> +		goto out;
>> +	}
>> +	path_prepend(path, "ata-%s", port_no);
>> +out:
>> +        udev_device_unref(atadev);
>> +        return parent;
>
> Indentation is borked... Needs to be 8ch all the way.

Sorry, usually use tabs, fixed that.

>
>> +}
>> +
>>   static struct udev_device *handle_scsi_default(struct udev_device *parent, char **path)
>>   {
>
> This is indication that the patch is not against git master, as this
> has been reindented a while back to put the bracket on the same line
> as the function name (see above). Please file these things as PRs,
> which will catch these issues right away, as the patch is then checked
> and test-built just by submitting it.

Ok, PR is against master.

Thanks for reviewing.

David