[dm-devel] [PATCH] 11-dm-mpath.rules: fix warnings reported by udevadm verify

[Martin Wilck]

On Mon, 2023-05-22 at 13:49 +0300, Dmitry V. Levin wrote:
> On Mon, May 22, 2023 at 12:19:04PM +0200, Martin Wilck wrote:
> > On Sat, 2023-05-20 at 01:33 +0300, Dmitry V. Levin wrote:
> > > Fix the following warnings reported by udevadm verify:
> > > 
> > > multipath/11-dm-mpath.rules:18 Whitespace after comma is
> > > expected.
> > > multipath/11-dm-mpath.rules:73 Whitespace after comma is
> > > expected.
> > > multipath/11-dm-mpath.rules:73 Whitespace after comma is
> > > expected.
> > > multipath/11-dm-mpath.rules:78 Whitespace after comma is
> > > expected.
> > > multipath/11-dm-mpath.rules:78 Whitespace after comma is
> > > expected.
> > > multipath/11-dm-mpath.rules:78 Whitespace after comma is
> > > expected.
> > > multipath/11-dm-mpath.rules: udev rules check failed
> > > 
> > > Signed-off-by: Dmitry V. Levin <ldv at strace.io>
> > 
> > Maybe you should have mentioned that you've just invented this
> > syntax
> > rule yourself (https://github.com/systemd/systemd/pull/26980).
> > I see no requirement for adding whitespace after a comma in the
> > udev
> > man page. 
> > 
> > Is this an attempt to change the udev rule syntax retroactively?
> 
> As you probably know, udevd silently accepts much broader syntax, for
> example, it doesn't need neither comma no whitespace between
> KEY=VALUE
> expressions, and I doubt this will ever change in the future.

Ok, that answers what I was asking. Thanks. Btw I did not know that
commas could be left out, I found out while I looked into this issue.

> In contrast, `udevadm verify` is a tool that checks syntactic,
> semantic,
> and style correctness of udev rules files.  

So this is about style and readability. Fair enough. It would have been
nice to mention that in the commit message.

> It indeed expects whitespace
> after a comma in udev rules - a style most of existing udev rules
> follow.

The multipath-tools rules do, too; they just don't add whitespace
between the comma and the line continuation marker '\'.

> but most of existing udev rules add whitespace
> between a comma and a backslash.

I see. I'll apply this patch then (and the other one with the missing
comma, too), but unless you object, I'll add a note to the
commit message explaining that this for improving readability 
and coding style compliance. I want to avoid the impression that the
existing code is technically wrong, which it isn't.

Reviewed-by: Martin Wilck <mwilck at suse.com>