Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Mon, Sep 30, 2019 at 8:35 AM Borislav Petkov wrote: > > On Tue, Sep 24, 2019 at 07:46:59AM +0300, Alexander Kapshuk wrote: > > gawk 5.0.1 generates the regexp warnings shown below: > > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape > > sequence `\:' is not a known regexp operator > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350: > > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape > > sequence `\&' is not a known regexp operator > > > > Ealier versions of gawk are not known to generate these warnings. > > > > The gawk manual referenced below does not list characters ':' and '&' > > as needing escaping, so 'unescape' them. > > https://www.gnu.org/software/gawk/manual/html_node/Escape-Sequences.html > > > > Running diff on the output generated by the script before and after > > applying the patch reported no differences. > > > > Signed-off-by: Alexander Kapshuk > > Reported-by: kbuild test robot > > Reviewed-by: Borislav Petkov > > This is not how Reviewed-by works. Read here: > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes > > for future reference. I fixed it up now. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette Thank you for taking in the patch and clarifying my misunderstanding about the use of 'Reviewed-by'. Point taken.
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Tue, Sep 24, 2019 at 07:46:59AM +0300, Alexander Kapshuk wrote: > gawk 5.0.1 generates the regexp warnings shown below: > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape > sequence `\:' is not a known regexp operator > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350: > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape > sequence `\&' is not a known regexp operator > > Ealier versions of gawk are not known to generate these warnings. > > The gawk manual referenced below does not list characters ':' and '&' > as needing escaping, so 'unescape' them. > https://www.gnu.org/software/gawk/manual/html_node/Escape-Sequences.html > > Running diff on the output generated by the script before and after > applying the patch reported no differences. > > Signed-off-by: Alexander Kapshuk > Reported-by: kbuild test robot > Reviewed-by: Borislav Petkov This is not how Reviewed-by works. Read here: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes for future reference. I fixed it up now. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Mon, 23 Sep 2019 12:31:39 +0200
Borislav Petkov wrote:
> + Masami.
>
> On Sun, Sep 22, 2019 at 06:03:28PM +0300, Alexander Kapshuk wrote:
> > This patch fixes the regexp warnings shown below:
>
> Avoid having "This patch" or "This commit" in the commit message. It is
> tautologically useless.
>
> Also, do
>
> $ git grep 'This patch' Documentation/process
>
> for more details.
>
> > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c
> > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape
> > sequence `\:' is not a known regexp operator
> > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350:
> > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape
> > sequence `\&' is not a known regexp operator
> >
> > The ':' and '&' characters need not escaping when used in string constants
> > as part of regular expressions.
>
> I could use a reasoning here, as in, "gawk manual doesn't have those two
> characters in the list here:
>
> https://www.gnu.org/software/gawk/manual/html_node/Escape-Sequences.html";
Thank you for pointing it out. It is good to refer this page as the
reason of this patch.
I couldn't remember why I added those escapes on those... (maybe for
compatibility with mawk? anyway, nowadays there seems no problem)
>
> or so.
>
> >
> > [Test-run]
> > awk -f arch/x86/tools/gen-insn-attr-x86.awk \
> > arch/x86/lib/x86-opcode-map.txt >../tmp/inat-tables.c
> >
> > diff arch/x86/lib/inat-tables.c ~/tmp/inat-tables.c; echo $?
> > 0
> >
> > awk -f tools/arch/x86/tools/gen-insn-attr-x86.awk \
> > tools/arch/x86/lib/x86-opcode-map.txt >../tmp/inat-tables.c
> >
> > diff tools/objtool/arch/x86/lib/inat-tables.c ~/tmp/inat-tables.c; echo $?
> > 0
>
> No need for that - just say that diffing the output before and after
> shows no changes.
>
> > [Debugging output]
> > DBG:ext:(66&F2)
> > DBG:match(ext, ...):(66&F2)
> > DBG:match(..., lprefix3_expr):\((F2|!F3|66&F2)\)
>
> That is supposed to say what exactly? That it still does what it is
> expected to do?
>
> Leaving in the rest for Masami.
This looks good to me, except for the description pointed above.
So feel free to add my ack on your patch on next version.
Acked-by: Masami Hiramatsu
Thank you,
>
> Thx.
>
> > Signed-off-by: Alexander Kapshuk
> > Reported-by: kbuild test robot
> > ---
> > arch/x86/tools/gen-insn-attr-x86.awk | 4 ++--
> > tools/arch/x86/tools/gen-insn-attr-x86.awk | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
> > b/arch/x86/tools/gen-insn-attr-x86.awk
> > index b02a36b2c14f..a42015b305f4 100644
> > --- a/arch/x86/tools/gen-insn-attr-x86.awk
> > +++ b/arch/x86/tools/gen-insn-attr-x86.awk
> > @@ -69,7 +69,7 @@ BEGIN {
> >
> > lprefix1_expr = "\\((66|!F3)\\)"
> > lprefix2_expr = "\\(F3\\)"
> > - lprefix3_expr = "\\((F2|!F3|66\\&F2)\\)"
> > + lprefix3_expr = "\\((F2|!F3|66&F2)\\)"
> > lprefix_expr = "\\((66|F2|F3)\\)"
> > max_lprefix = 4
> >
> > @@ -257,7 +257,7 @@ function convert_operands(count,opnd, i,j,imm,mod)
> > return add_flags(imm, mod)
> > }
> >
> > -/^[0-9a-f]+\:/ {
> > +/^[0-9a-f]+:/ {
> > if (NR == 1)
> > next
> > # get index
> > diff --git a/tools/arch/x86/tools/gen-insn-attr-x86.awk
> > b/tools/arch/x86/tools/gen-insn-attr-x86.awk
> > index b02a36b2c14f..a42015b305f4 100644
> > --- a/tools/arch/x86/tools/gen-insn-attr-x86.awk
> > +++ b/tools/arch/x86/tools/gen-insn-attr-x86.awk
> > @@ -69,7 +69,7 @@ BEGIN {
> >
> > lprefix1_expr = "\\((66|!F3)\\)"
> > lprefix2_expr = "\\(F3\\)"
> > - lprefix3_expr = "\\((F2|!F3|66\\&F2)\\)"
> > + lprefix3_expr = "\\((F2|!F3|66&F2)\\)"
> > lprefix_expr = "\\((66|F2|F3)\\)"
> > max_lprefix = 4
> >
> > @@ -257,7 +257,7 @@ function convert_operands(count,opnd, i,j,imm,mod)
> > return add_flags(imm, mod)
> > }
> >
> > -/^[0-9a-f]+\:/ {
> > +/^[0-9a-f]+:/ {
> > if (NR == 1)
> > next
> > # get index
> > --
> > 2.23.0
> >
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
--
Masami Hiramatsu
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Mon, Sep 23, 2019 at 1:31 PM Borislav Petkov wrote: > > + Masami. > > On Sun, Sep 22, 2019 at 06:03:28PM +0300, Alexander Kapshuk wrote: > > This patch fixes the regexp warnings shown below: > > Avoid having "This patch" or "This commit" in the commit message. It is > tautologically useless. > > Also, do > > $ git grep 'This patch' Documentation/process > > for more details. > > > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape > > sequence `\:' is not a known regexp operator > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350: > > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape > > sequence `\&' is not a known regexp operator > > > > The ':' and '&' characters need not escaping when used in string constants > > as part of regular expressions. > > I could use a reasoning here, as in, "gawk manual doesn't have those two > characters in the list here: > > https://www.gnu.org/software/gawk/manual/html_node/Escape-Sequences.html"; > > or so. > > > > > [Test-run] > > awk -f arch/x86/tools/gen-insn-attr-x86.awk \ > > arch/x86/lib/x86-opcode-map.txt >../tmp/inat-tables.c > > > > diff arch/x86/lib/inat-tables.c ~/tmp/inat-tables.c; echo $? > > 0 > > > > awk -f tools/arch/x86/tools/gen-insn-attr-x86.awk \ > > tools/arch/x86/lib/x86-opcode-map.txt >../tmp/inat-tables.c > > > > diff tools/objtool/arch/x86/lib/inat-tables.c ~/tmp/inat-tables.c; echo $? > > 0 > > No need for that - just say that diffing the output before and after > shows no changes. > > > [Debugging output] > > DBG:ext:(66&F2) > > DBG:match(ext, ...):(66&F2) > > DBG:match(..., lprefix3_expr):\((F2|!F3|66&F2)\) > > That is supposed to say what exactly? That it still does what it is > expected to do? That was the intention. Thanks for reviewing the patch. I'll wait to hear from Masami before resending the patch. > > Leaving in the rest for Masami. > > Thx. > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
+ Masami.
On Sun, Sep 22, 2019 at 06:03:28PM +0300, Alexander Kapshuk wrote:
> This patch fixes the regexp warnings shown below:
Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.
Also, do
$ git grep 'This patch' Documentation/process
for more details.
> GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c
> awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape
> sequence `\:' is not a known regexp operator
> awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350:
> (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape
> sequence `\&' is not a known regexp operator
>
> The ':' and '&' characters need not escaping when used in string constants
> as part of regular expressions.
I could use a reasoning here, as in, "gawk manual doesn't have those two
characters in the list here:
https://www.gnu.org/software/gawk/manual/html_node/Escape-Sequences.html";
or so.
>
> [Test-run]
> awk -f arch/x86/tools/gen-insn-attr-x86.awk \
> arch/x86/lib/x86-opcode-map.txt >../tmp/inat-tables.c
>
> diff arch/x86/lib/inat-tables.c ~/tmp/inat-tables.c; echo $?
> 0
>
> awk -f tools/arch/x86/tools/gen-insn-attr-x86.awk \
> tools/arch/x86/lib/x86-opcode-map.txt >../tmp/inat-tables.c
>
> diff tools/objtool/arch/x86/lib/inat-tables.c ~/tmp/inat-tables.c; echo $?
> 0
No need for that - just say that diffing the output before and after
shows no changes.
> [Debugging output]
> DBG:ext:(66&F2)
> DBG:match(ext, ...):(66&F2)
> DBG:match(..., lprefix3_expr):\((F2|!F3|66&F2)\)
That is supposed to say what exactly? That it still does what it is
expected to do?
Leaving in the rest for Masami.
Thx.
> Signed-off-by: Alexander Kapshuk
> Reported-by: kbuild test robot
> ---
> arch/x86/tools/gen-insn-attr-x86.awk | 4 ++--
> tools/arch/x86/tools/gen-insn-attr-x86.awk | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/tools/gen-insn-attr-x86.awk
> b/arch/x86/tools/gen-insn-attr-x86.awk
> index b02a36b2c14f..a42015b305f4 100644
> --- a/arch/x86/tools/gen-insn-attr-x86.awk
> +++ b/arch/x86/tools/gen-insn-attr-x86.awk
> @@ -69,7 +69,7 @@ BEGIN {
>
> lprefix1_expr = "\\((66|!F3)\\)"
> lprefix2_expr = "\\(F3\\)"
> - lprefix3_expr = "\\((F2|!F3|66\\&F2)\\)"
> + lprefix3_expr = "\\((F2|!F3|66&F2)\\)"
> lprefix_expr = "\\((66|F2|F3)\\)"
> max_lprefix = 4
>
> @@ -257,7 +257,7 @@ function convert_operands(count,opnd, i,j,imm,mod)
> return add_flags(imm, mod)
> }
>
> -/^[0-9a-f]+\:/ {
> +/^[0-9a-f]+:/ {
> if (NR == 1)
> next
> # get index
> diff --git a/tools/arch/x86/tools/gen-insn-attr-x86.awk
> b/tools/arch/x86/tools/gen-insn-attr-x86.awk
> index b02a36b2c14f..a42015b305f4 100644
> --- a/tools/arch/x86/tools/gen-insn-attr-x86.awk
> +++ b/tools/arch/x86/tools/gen-insn-attr-x86.awk
> @@ -69,7 +69,7 @@ BEGIN {
>
> lprefix1_expr = "\\((66|!F3)\\)"
> lprefix2_expr = "\\(F3\\)"
> - lprefix3_expr = "\\((F2|!F3|66\\&F2)\\)"
> + lprefix3_expr = "\\((F2|!F3|66&F2)\\)"
> lprefix_expr = "\\((66|F2|F3)\\)"
> max_lprefix = 4
>
> @@ -257,7 +257,7 @@ function convert_operands(count,opnd, i,j,imm,mod)
> return add_flags(imm, mod)
> }
>
> -/^[0-9a-f]+\:/ {
> +/^[0-9a-f]+:/ {
> if (NR == 1)
> next
> # get index
> --
> 2.23.0
>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Mon, Sep 23, 2019 at 12:56:14PM +0300, Alexander Kapshuk wrote: > On Mon, Sep 23, 2019 at 12:19 PM Borislav Petkov wrote: > > > > On Sun, Sep 22, 2019 at 06:03:28PM +0300, Alexander Kapshuk wrote: > > > This patch fixes the regexp warnings shown below: > > > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c > > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape > > > sequence `\:' is not a known regexp operator > > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350: > > > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp > > > escape sequence `\&' is not a known regexp operator > > > > > > The ':' and '&' characters need not escaping when used in string constants > > > as part of regular expressions. > > > > How do you trigger this? > > > > I don't see it in my builds so it looks like environment thing. What > > flavor of awk is yours? > > > > Thx. > > > > -- > > Regards/Gruss, > > Boris. > > > > https://people.kernel.org/tglx/notes-about-netiquette > > gawk 5.0.1-1 on Arch Linux. Ok, looks like they're enforcing it in v5 as v4 which I have doesn't complain. Please add that fact to the commit message. More review as a reply to your second patch. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Mon, Sep 23, 2019 at 12:19 PM Borislav Petkov wrote: > > On Sun, Sep 22, 2019 at 06:03:28PM +0300, Alexander Kapshuk wrote: > > This patch fixes the regexp warnings shown below: > > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape > > sequence `\:' is not a known regexp operator > > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350: > > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape > > sequence `\&' is not a known regexp operator > > > > The ':' and '&' characters need not escaping when used in string constants > > as part of regular expressions. > > How do you trigger this? > > I don't see it in my builds so it looks like environment thing. What > flavor of awk is yours? > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette gawk 5.0.1-1 on Arch Linux.
Re: [PATCH RESEND] gen-insn-attr-x86.awk: Fix regexp warnings
On Sun, Sep 22, 2019 at 06:03:28PM +0300, Alexander Kapshuk wrote: > This patch fixes the regexp warnings shown below: > GEN /home/sasha/torvalds/tools/objtool/arch/x86/lib/inat-tables.c > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:260: warning: regexp escape > sequence `\:' is not a known regexp operator > awk: ../arch/x86/tools/gen-insn-attr-x86.awk:350: > (FILENAME=../arch/x86/lib/x86-opcode-map.txt FNR=41) warning: regexp escape > sequence `\&' is not a known regexp operator > > The ':' and '&' characters need not escaping when used in string constants > as part of regular expressions. How do you trigger this? I don't see it in my builds so it looks like environment thing. What flavor of awk is yours? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
