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

[Lennart Poettering]

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

>  
> +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.

> +
> +	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.

> +}
> +
>  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.

Thanks,

Lennart

-- 
Lennart Poettering, Red Hat