{"thread":{"id":"52052","subject":"[PATCH v2] userdiff: Fix some corner cases in dts regex","startedAt":"2019-10-16T20:32:43Z","lastAt":"2019-10-20T18:27:12Z","messageCount":3,"participants":["Stephen Boyd","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"384213","messageId":"20191016203239.212174-1-sboyd@kernel.org","threadId":"52052","inReplyTo":null,"subject":"[PATCH v2] userdiff: Fix some corner cases in dts regex","fromName":"Stephen Boyd","fromEmail":"sboyd@kernel.org","sentAt":"2019-10-16T20:32:39Z","receivedAt":"2019-10-16T20:32:43Z","isPatch":true,"sender":{"key":"sboyd@kernel.org","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"While reviewing some dts diffs recently I noticed that the hunk header\nlogic was failing to find the containing node. This is because the regex\ndoesn't consider properties that may span multiple lines, i.e.\n\n\tproperty = ,\n\t\t ;\n\nand it got hung up on comments inside nodes that look like the root node\nbecause they start with '/*'. Add tests for these cases and update the\nregex to find them. Maybe detecting the root node is too complicated but\nforcing it to be a backslash with any amount of whitespace up to an open\nbracket seemed OK. I tried to detect that a comment is in-between the\ntwo parts but I wasn't happy so I just dropped it.\n\nCc: Rob Herring \nCc: Frank Rowand \nSigned-off-by: Stephen Boyd \n---\n\nChanges from v1:\n * Added a new boolean property unit test\n * Updated the regex to simplify multi-line property skipping\n * Added some space to multieline prop test between cells in\n first property\n\n t/t4018/dts-nodes-boolean-prop | 9 +++++++++\n t/t4018/dts-nodes-multiline-prop | 14 ++++++++++++++\n t/t4018/dts-root | 2 +-\n t/t4018/dts-root-comment | 8 ++++++++\n userdiff.c | 3 ++-\n 5 files changed, 34 insertions(+), 2 deletions(-)\n create mode 100644 t/t4018/dts-nodes-boolean-prop\n create mode 100644 t/t4018/dts-nodes-multiline-prop\n create mode 100644 t/t4018/dts-root-comment\n\ndiff --git a/t/t4018/dts-nodes-boolean-prop b/t/t4018/dts-nodes-boolean-prop\nnew file mode 100644\nindex 000000000000..afc6b5b404e4\n--- /dev/null\n+++ b/t/t4018/dts-nodes-boolean-prop\n@@ -0,0 +1,9 @@\n+/ {\n+\tlabel_1: node1@ff00 {\n+\t\tRIGHT@deadf00,4000 {\n+\t\t\tboolean-prop1;\n+\n+\t\t\tChangeMe;\n+\t\t};\n+\t};\n+};\ndiff --git a/t/t4018/dts-nodes-multiline-prop b/t/t4018/dts-nodes-multiline-prop\nnew file mode 100644\nindex 000000000000..db4b4bdda686\n--- /dev/null\n+++ b/t/t4018/dts-nodes-multiline-prop\n@@ -0,0 +1,14 @@\n+/ {\n+\tlabel_1: node1@ff00 {\n+\t\tRIGHT@deadf00,4000 {\n+\t\t\tmultilineprop = <3>,\n+\n+\n+\t\t\t\t\t<4>;\n+\n+\n+\n+\t\t\tChangeMe = <0xffeedd00>;\n+\t\t};\n+\t};\n+};\ndiff --git a/t/t4018/dts-root b/t/t4018/dts-root\nindex 2ef9e6ffaa2c..4353b8220c91 100644\n--- a/t/t4018/dts-root\n+++ b/t/t4018/dts-root\n@@ -1,4 +1,4 @@\n-/RIGHT { /* Technically just supposed to be a slash */\n+/ { RIGHT /* Technically just supposed to be a slash and brace */\n \t#size-cells = <1>;\n \n \tChangeMe = <0xffeedd00>;\ndiff --git a/t/t4018/dts-root-comment b/t/t4018/dts-root-comment\nnew file mode 100644\nindex 000000000000..333a625c7007\n--- /dev/null\n+++ b/t/t4018/dts-root-comment\n@@ -0,0 +1,8 @@\n+/ { RIGHT /* Technically just supposed to be a slash and brace */\n+\t#size-cells = <1>;\n+\n+\t/* This comment should be ignored */\n+\n+\tsome-property = <40+2>;\n+\tChangeMe = <0xffeedd00>;\n+};\ndiff --git a/userdiff.c b/userdiff.c\nindex 86e3244e15dd..e187d356f6ff 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -25,8 +25,9 @@ IPATTERN(\"ada\",\n \t \"|=>|\\\\.\\\\.|\\\\*\\\\*|:=|/=|>=|<=|<<|>>|<>\"),\n PATTERNS(\"dts\",\n \t \"!;\\n\"\n+\t \"!=\\n\"\n \t /* lines beginning with a word optionally preceded by '&' or the root */\n-\t \"^[ \\t]*((/|&?[a-zA-Z_]).*)\",\n+\t \"^[ \\t]*((/[ \\t]*\\\\{|&?[a-zA-Z_]).*)\",\n \t /* -- */\n \t /* Property names and math operators */\n \t \"[a-zA-Z0-9,._+?#-]+\"\n-- \nSent by a computer through tubes\n\n"},{"id":"384216","messageId":"4f8e8a13-c41f-558f-18c3-b2bda178a06b@kdbg.org","threadId":"52052","inReplyTo":"20191016203239.212174-1-sboyd@kernel.org","subject":"Re: [PATCH v2] userdiff: Fix some corner cases in dts regex","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-10-16T21:10:09Z","receivedAt":"2019-10-16T21:10:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"[Removed bouncing addresses of Matthieu Moy and William Duclot from Cc]\n\nAm 16.10.19 um 22:32 schrieb Stephen Boyd:\n> diff --git a/t/t4018/dts-nodes-multiline-prop b/t/t4018/dts-nodes-multiline-prop\n> new file mode 100644\n> index 000000000000..db4b4bdda686\n> --- /dev/null\n> +++ b/t/t4018/dts-nodes-multiline-prop\n> @@ -0,0 +1,14 @@\n> +/ {\n> +\tlabel_1: node1@ff00 {\n> +\t\tRIGHT@deadf00,4000 {\n> +\t\t\tmultilineprop = <3>,\n> +\n> +\n> +\t\t\t\t\t<4>;\n\nI was actually thinking about something like\n\n\t\t\tmultilineprop = <3>,\n\t\t\t\t\t<0xabcd>,\n\t\t\t\t\t\"text\",\n\t\t\t\t\tname,\n\t\t\t\t\t<4>;\n\nor something like that -- whatever occurs in the real world.\n\n> +\n> +\n> +\n> +\t\t\tChangeMe = <0xffeedd00>;\n> +\t\t};\n> +\t};\n> +};\n\nApart from that, the patch looks good.\n\n-- Hannes\n"},{"id":"384475","messageId":"20191020182709.CC74A218BA@mail.kernel.org","threadId":"52052","inReplyTo":"4f8e8a13-c41f-558f-18c3-b2bda178a06b@kdbg.org","subject":"Re: [PATCH v2] userdiff: Fix some corner cases in dts regex","fromName":"Stephen Boyd","fromEmail":"sboyd@kernel.org","sentAt":"2019-10-20T18:27:08Z","receivedAt":"2019-10-20T18:27:12Z","isPatch":true,"sender":{"key":"sboyd@kernel.org","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Quoting Johannes Sixt (2019-10-16 14:10:09)\n> [Removed bouncing addresses of Matthieu Moy and William Duclot from Cc]\n> \n> Am 16.10.19 um 22:32 schrieb Stephen Boyd:\n> > diff --git a/t/t4018/dts-nodes-multiline-prop b/t/t4018/dts-nodes-multiline-prop\n> > new file mode 100644\n> > index 000000000000..db4b4bdda686\n> > --- /dev/null\n> > +++ b/t/t4018/dts-nodes-multiline-prop\n> > @@ -0,0 +1,14 @@\n> > +/ {\n> > + label_1: node1@ff00 {\n> > + RIGHT@deadf00,4000 {\n> > + multilineprop = <3>,\n> > +\n> > +\n> > + <4>;\n> \n> I was actually thinking about something like\n> \n> multilineprop = <3>,\n> <0xabcd>,\n> \"text\",\n> name,\n> <4>;\n> \n> or something like that -- whatever occurs in the real world.\n> \n\nOk sure. I can have a list of numbers that spans four or five lines.\n\n> > +\n> > +\n> > +\n> > + ChangeMe = <0xffeedd00>;\n> > + };\n> > + };\n> > +};\n> \n> Apart from that, the patch looks good.\n> \n\nCool. I'll resend.\n\n"}]}