[RHEL7 qemu-kvm PATCH 0/3] Fixing several GCC 11 warnings

[RHEL7 qemu-kvm PATCH 0/3] Fixing several GCC 11 warnings

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

Compiling qemu using GCC 11 we got several new warnings. To allow
build with --enable-werror, we need to solve issues generating these
warnings.

Signed-of-by: Miroslav Rezanina <mrezanin@redhat.com>

Miroslav Rezanina (3):
  Fix net.c warning on GCC 11
  s390x: Fix vm name copy length
  Fix tcg_out_op argument mismatch warning

 net/eth.c                    | 3 +++
 target/s390x/kvm.c           | 2 +-
 target/s390x/misc_helper.c   | 4 +++-
 tcg/aarch64/tcg-target.c.inc | 3 +--
 tcg/sparc/tcg-target.c.inc   | 3 +--
 5 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.18.4

[RHEL7 qemu-kvm PATCH 1/3] Fix net.c warning on GCC 11

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

When building qemu with GCC 11, compiling eth.c file produce following warning:

   warning: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Warray-bounds]

This caused by retyping from ip6_ext_hdr to ip6_ext_hdr_routing that has more
attributes.

As this usage is expected, suppress the warning temporarily through the function
using this retyping.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 net/eth.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/eth.c b/net/eth.c
index 1e0821c5f8..b9bdd0435c 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -405,6 +405,8 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
                         struct ip6_ext_hdr *ext_hdr,
                         struct in6_address *dst_addr)
 {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
     struct ip6_ext_hdr_routing *rthdr = (struct ip6_ext_hdr_routing *) ext_hdr;
 
     if ((rthdr->rtype == 2) &&
@@ -426,6 +428,7 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int pkt_frags,
     }
 
     return false;
+#pragma GCC diagnostic pop
 }
 
 static bool
-- 
2.18.4

[RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

There are two cases when vm name is copied but closing \0 can be lost
in case name is too long (>=256 characters).

Updating length to copy so there is space for closing \0.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 target/s390x/kvm.c         | 2 +-
 target/s390x/misc_helper.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index b8385e6b95..2313b5727e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
      */
     if (qemu_name) {
         strncpy((char *)sysib.ext_names[0], qemu_name,
-                sizeof(sysib.ext_names[0]));
+                sizeof(sysib.ext_names[0]) - 1);
     } else {
         strcpy((char *)sysib.ext_names[0], "KVMguest");
     }
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 58dbc023eb..7c478b9e58 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -369,8 +369,10 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, uint64_t r0, uint64_t r1)
                 ebcdic_put(sysib.sysib_322.vm[0].name, qemu_name,
                            MIN(sizeof(sysib.sysib_322.vm[0].name),
                                strlen(qemu_name)));
+		memset((char *)sysib.sysib_322.ext_names[0], 0, 
+		       sizeof(sysib.sysib_322.ext_names[0]));
                 strncpy((char *)sysib.sysib_322.ext_names[0], qemu_name,
-                        sizeof(sysib.sysib_322.ext_names[0]));
+                        sizeof(sysib.sysib_322.ext_names[0]) - 1);
             } else {
                 ebcdic_put(sysib.sysib_322.vm[0].name, "TCGguest", 8);
                 strcpy((char *)sysib.sysib_322.ext_names[0], "TCGguest");
-- 
2.18.4

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Philippe Mathieu-Daudé]

Hi Miroslav,

On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> There are two cases when vm name is copied but closing \0 can be lost
> in case name is too long (>=256 characters).
> 
> Updating length to copy so there is space for closing \0.
> 
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> ---
>  target/s390x/kvm.c         | 2 +-
>  target/s390x/misc_helper.c | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index b8385e6b95..2313b5727e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>       */
>      if (qemu_name) {
>          strncpy((char *)sysib.ext_names[0], qemu_name,
> -                sizeof(sysib.ext_names[0]));
> +                sizeof(sysib.ext_names[0]) - 1);
>      } else {
>          strcpy((char *)sysib.ext_names[0], "KVMguest");
>      }

What about using strpadcpy() instead?

    strpadcpy((char *)sysib.sysib_322.ext_names[0],
              sizeof(sysib.sysib_322.ext_names[0]),
              qemu_name ?: "KVMguest", '\0');

> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index 58dbc023eb..7c478b9e58 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -369,8 +369,10 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, uint64_t r0, uint64_t r1)
>                  ebcdic_put(sysib.sysib_322.vm[0].name, qemu_name,
>                             MIN(sizeof(sysib.sysib_322.vm[0].name),
>                                 strlen(qemu_name)));
> +		memset((char *)sysib.sysib_322.ext_names[0], 0, 
> +		       sizeof(sysib.sysib_322.ext_names[0]));
>                  strncpy((char *)sysib.sysib_322.ext_names[0], qemu_name,
> -                        sizeof(sysib.sysib_322.ext_names[0]));
> +                        sizeof(sysib.sysib_322.ext_names[0]) - 1);

And here:

               strpadcpy((char *)sysib.sysib_322.ext_names[0],
                         sizeof(sysib.sysib_322.ext_names[0]),
                         qemu_name, '\0');

>              } else {
>                  ebcdic_put(sysib.sysib_322.vm[0].name, "TCGguest", 8);
>                  strcpy((char *)sysib.sysib_322.ext_names[0], "TCGguest");
> 

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Thomas Huth]

On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
> Hi Miroslav,
> 
> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>
>> There are two cases when vm name is copied but closing \0 can be lost
>> in case name is too long (>=256 characters).
>>
>> Updating length to copy so there is space for closing \0.
>>
>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
>> ---
>>   target/s390x/kvm.c         | 2 +-
>>   target/s390x/misc_helper.c | 4 +++-
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index b8385e6b95..2313b5727e 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>>        */
>>       if (qemu_name) {
>>           strncpy((char *)sysib.ext_names[0], qemu_name,
>> -                sizeof(sysib.ext_names[0]));
>> +                sizeof(sysib.ext_names[0]) - 1);
>>       } else {
>>           strcpy((char *)sysib.ext_names[0], "KVMguest");
>>       }
> 
> What about using strpadcpy() instead?

Yes, strpadcpy is the better way here - this field has to be padded with 
zeroes, so doing "- 1" is wrong here.

  Thomas

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Miroslav Rezanina]

----- Original Message -----
> From: "Thomas Huth" <thuth@redhat.com>
> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com, qemu-devel@nongnu.org, "qemu-s390x"
> <qemu-s390x@nongnu.org>
> Sent: Monday, January 11, 2021 1:24:57 PM
> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
> 
> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
> > Hi Miroslav,
> > 
> > On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> >> From: Miroslav Rezanina <mrezanin@redhat.com>
> >>
> >> There are two cases when vm name is copied but closing \0 can be lost
> >> in case name is too long (>=256 characters).
> >>
> >> Updating length to copy so there is space for closing \0.
> >>
> >> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> >> ---
> >>   target/s390x/kvm.c         | 2 +-
> >>   target/s390x/misc_helper.c | 4 +++-
> >>   2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> >> index b8385e6b95..2313b5727e 100644
> >> --- a/target/s390x/kvm.c
> >> +++ b/target/s390x/kvm.c
> >> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
> >> addr, uint8_t ar)
> >>        */
> >>       if (qemu_name) {
> >>           strncpy((char *)sysib.ext_names[0], qemu_name,
> >> -                sizeof(sysib.ext_names[0]));
> >> +                sizeof(sysib.ext_names[0]) - 1);
> >>       } else {
> >>           strcpy((char *)sysib.ext_names[0], "KVMguest");
> >>       }
> > 
> > What about using strpadcpy() instead?
> 
> Yes, strpadcpy is the better way here - this field has to be padded with
> zeroes, so doing "- 1" is wrong here.

Hi Thomas,

as I wrote in reply to Phillipe - the array is memset to zeroes before the if so we
are sure it's padded with zeroes (in this occurrence, not true for second one).

Mirek

> 
>   Thomas
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Thomas Huth]

On 11/01/2021 13.42, Miroslav Rezanina wrote:
> 
> 
> ----- Original Message -----
>> From: "Thomas Huth" <thuth@redhat.com>
>> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com, qemu-devel@nongnu.org, "qemu-s390x"
>> <qemu-s390x@nongnu.org>
>> Sent: Monday, January 11, 2021 1:24:57 PM
>> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
>>
>> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
>>> Hi Miroslav,
>>>
>>> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
>>>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>>>
>>>> There are two cases when vm name is copied but closing \0 can be lost
>>>> in case name is too long (>=256 characters).
>>>>
>>>> Updating length to copy so there is space for closing \0.
>>>>
>>>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
>>>> ---
>>>>    target/s390x/kvm.c         | 2 +-
>>>>    target/s390x/misc_helper.c | 4 +++-
>>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>> index b8385e6b95..2313b5727e 100644
>>>> --- a/target/s390x/kvm.c
>>>> +++ b/target/s390x/kvm.c
>>>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
>>>> addr, uint8_t ar)
>>>>         */
>>>>        if (qemu_name) {
>>>>            strncpy((char *)sysib.ext_names[0], qemu_name,
>>>> -                sizeof(sysib.ext_names[0]));
>>>> +                sizeof(sysib.ext_names[0]) - 1);
>>>>        } else {
>>>>            strcpy((char *)sysib.ext_names[0], "KVMguest");
>>>>        }
>>>
>>> What about using strpadcpy() instead?
>>
>> Yes, strpadcpy is the better way here - this field has to be padded with
>> zeroes, so doing "- 1" is wrong here.
> 
> Hi Thomas,
> 
> as I wrote in reply to Phillipe - the array is memset to zeroes before the if so we
> are sure it's padded with zeroes (in this occurrence, not true for second one).

Ok, but dropping the last character is still wrong here. The ext_names do 
not need to be terminated with a \0 if they have the full length.

  Thomas

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Miroslav Rezanina]

----- Original Message -----
> From: "Thomas Huth" <thuth@redhat.com>
> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> Cc: "qemu-s390x" <qemu-s390x@nongnu.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
> Sent: Monday, January 11, 2021 1:54:06 PM
> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
> 
> On 11/01/2021 13.42, Miroslav Rezanina wrote:
> > 
> > 
> > ----- Original Message -----
> >> From: "Thomas Huth" <thuth@redhat.com>
> >> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com,
> >> qemu-devel@nongnu.org, "qemu-s390x"
> >> <qemu-s390x@nongnu.org>
> >> Sent: Monday, January 11, 2021 1:24:57 PM
> >> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
> >>
> >> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
> >>> Hi Miroslav,
> >>>
> >>> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> >>>> From: Miroslav Rezanina <mrezanin@redhat.com>
> >>>>
> >>>> There are two cases when vm name is copied but closing \0 can be lost
> >>>> in case name is too long (>=256 characters).
> >>>>
> >>>> Updating length to copy so there is space for closing \0.
> >>>>
> >>>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> >>>> ---
> >>>>    target/s390x/kvm.c         | 2 +-
> >>>>    target/s390x/misc_helper.c | 4 +++-
> >>>>    2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> >>>> index b8385e6b95..2313b5727e 100644
> >>>> --- a/target/s390x/kvm.c
> >>>> +++ b/target/s390x/kvm.c
> >>>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
> >>>> addr, uint8_t ar)
> >>>>         */
> >>>>        if (qemu_name) {
> >>>>            strncpy((char *)sysib.ext_names[0], qemu_name,
> >>>> -                sizeof(sysib.ext_names[0]));
> >>>> +                sizeof(sysib.ext_names[0]) - 1);
> >>>>        } else {
> >>>>            strcpy((char *)sysib.ext_names[0], "KVMguest");
> >>>>        }
> >>>
> >>> What about using strpadcpy() instead?
> >>
> >> Yes, strpadcpy is the better way here - this field has to be padded with
> >> zeroes, so doing "- 1" is wrong here.
> > 
> > Hi Thomas,
> > 
> > as I wrote in reply to Phillipe - the array is memset to zeroes before the
> > if so we
> > are sure it's padded with zeroes (in this occurrence, not true for second
> > one).
> 
> Ok, but dropping the last character is still wrong here. The ext_names do
> not need to be terminated with a \0 if they have the full length.

Oh, they do not have to end with \0??? I did not know it. In that case we
probably have to use strpadcpy to get rid of the compiler warning.

Mirek
> 
>   Thomas
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Christian Borntraeger]

On 11.01.21 13:54, Thomas Huth wrote:
> On 11/01/2021 13.42, Miroslav Rezanina wrote:
>>
>>
>> ----- Original Message -----
>>> From: "Thomas Huth" <thuth@redhat.com>
>>> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com, qemu-devel@nongnu.org, "qemu-s390x"
>>> <qemu-s390x@nongnu.org>
>>> Sent: Monday, January 11, 2021 1:24:57 PM
>>> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
>>>
>>> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
>>>> Hi Miroslav,
>>>>
>>>> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
>>>>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>>>>
>>>>> There are two cases when vm name is copied but closing \0 can be lost
>>>>> in case name is too long (>=256 characters).
>>>>>
>>>>> Updating length to copy so there is space for closing \0.
>>>>>
>>>>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
>>>>> ---
>>>>>    target/s390x/kvm.c         | 2 +-
>>>>>    target/s390x/misc_helper.c | 4 +++-
>>>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>>> index b8385e6b95..2313b5727e 100644
>>>>> --- a/target/s390x/kvm.c
>>>>> +++ b/target/s390x/kvm.c
>>>>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
>>>>> addr, uint8_t ar)
>>>>>         */
>>>>>        if (qemu_name) {
>>>>>            strncpy((char *)sysib.ext_names[0], qemu_name,
>>>>> -                sizeof(sysib.ext_names[0]));
>>>>> +                sizeof(sysib.ext_names[0]) - 1);
>>>>>        } else {
>>>>>            strcpy((char *)sysib.ext_names[0], "KVMguest");
>>>>>        }
>>>>
>>>> What about using strpadcpy() instead?
>>>
>>> Yes, strpadcpy is the better way here - this field has to be padded with
>>> zeroes, so doing "- 1" is wrong here.
>>
>> Hi Thomas,
>>
>> as I wrote in reply to Phillipe - the array is memset to zeroes before the if so we
>> are sure it's padded with zeroes (in this occurrence, not true for second one).
> 
> Ok, but dropping the last character is still wrong here. The ext_names do not need to be terminated with a \0 if they have the full length.
The current code is actually correct. We are perfectly fine without the final \n if the string is really 256 bytes.

Replacing memset + strncpy with strpadcpy is certainly a good cleanup. Is it necessary? No.

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Christian Borntraeger]

On 11.01.21 14:02, Christian Borntraeger wrote:
> 
> 
> On 11.01.21 13:54, Thomas Huth wrote:
>> On 11/01/2021 13.42, Miroslav Rezanina wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Thomas Huth" <thuth@redhat.com>
>>>> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com, qemu-devel@nongnu.org, "qemu-s390x"
>>>> <qemu-s390x@nongnu.org>
>>>> Sent: Monday, January 11, 2021 1:24:57 PM
>>>> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
>>>>
>>>> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
>>>>> Hi Miroslav,
>>>>>
>>>>> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
>>>>>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>>>>>
>>>>>> There are two cases when vm name is copied but closing \0 can be lost
>>>>>> in case name is too long (>=256 characters).
>>>>>>
>>>>>> Updating length to copy so there is space for closing \0.
>>>>>>
>>>>>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
>>>>>> ---
>>>>>>    target/s390x/kvm.c         | 2 +-
>>>>>>    target/s390x/misc_helper.c | 4 +++-
>>>>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>>>> index b8385e6b95..2313b5727e 100644
>>>>>> --- a/target/s390x/kvm.c
>>>>>> +++ b/target/s390x/kvm.c
>>>>>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
>>>>>> addr, uint8_t ar)
>>>>>>         */
>>>>>>        if (qemu_name) {
>>>>>>            strncpy((char *)sysib.ext_names[0], qemu_name,
>>>>>> -                sizeof(sysib.ext_names[0]));
>>>>>> +                sizeof(sysib.ext_names[0]) - 1);
>>>>>>        } else {
>>>>>>            strcpy((char *)sysib.ext_names[0], "KVMguest");
>>>>>>        }
>>>>>
>>>>> What about using strpadcpy() instead?
>>>>
>>>> Yes, strpadcpy is the better way here - this field has to be padded with
>>>> zeroes, so doing "- 1" is wrong here.
>>>
>>> Hi Thomas,
>>>
>>> as I wrote in reply to Phillipe - the array is memset to zeroes before the if so we
>>> are sure it's padded with zeroes (in this occurrence, not true for second one).
>>
>> Ok, but dropping the last character is still wrong here. The ext_names do not need to be terminated with a \0 if they have the full length.
> The current code is actually correct. We are perfectly fine without the final \n if the string is really 256 bytes.
> 
> Replacing memset + strncpy with strpadcpy is certainly a good cleanup. Is it necessary? No.

And yes, Thomas is right. The -1 is wrong. 

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Miroslav Rezanina]

----- Original Message -----
> From: "Christian Borntraeger" <borntraeger@de.ibm.com>
> To: "Thomas Huth" <thuth@redhat.com>, "Miroslav Rezanina" <mrezanin@redhat.com>
> Cc: "qemu-s390x" <qemu-s390x@nongnu.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
> Sent: Monday, January 11, 2021 2:02:32 PM
> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
> 
> 
> 
> On 11.01.21 13:54, Thomas Huth wrote:
> > On 11/01/2021 13.42, Miroslav Rezanina wrote:
> >>
> >>
> >> ----- Original Message -----
> >>> From: "Thomas Huth" <thuth@redhat.com>
> >>> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com,
> >>> qemu-devel@nongnu.org, "qemu-s390x"
> >>> <qemu-s390x@nongnu.org>
> >>> Sent: Monday, January 11, 2021 1:24:57 PM
> >>> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
> >>>
> >>> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
> >>>> Hi Miroslav,
> >>>>
> >>>> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> >>>>> From: Miroslav Rezanina <mrezanin@redhat.com>
> >>>>>
> >>>>> There are two cases when vm name is copied but closing \0 can be lost
> >>>>> in case name is too long (>=256 characters).
> >>>>>
> >>>>> Updating length to copy so there is space for closing \0.
> >>>>>
> >>>>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> >>>>> ---
> >>>>>    target/s390x/kvm.c         | 2 +-
> >>>>>    target/s390x/misc_helper.c | 4 +++-
> >>>>>    2 files changed, 4 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> >>>>> index b8385e6b95..2313b5727e 100644
> >>>>> --- a/target/s390x/kvm.c
> >>>>> +++ b/target/s390x/kvm.c
> >>>>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
> >>>>> addr, uint8_t ar)
> >>>>>         */
> >>>>>        if (qemu_name) {
> >>>>>            strncpy((char *)sysib.ext_names[0], qemu_name,
> >>>>> -                sizeof(sysib.ext_names[0]));
> >>>>> +                sizeof(sysib.ext_names[0]) - 1);
> >>>>>        } else {
> >>>>>            strcpy((char *)sysib.ext_names[0], "KVMguest");
> >>>>>        }
> >>>>
> >>>> What about using strpadcpy() instead?
> >>>
> >>> Yes, strpadcpy is the better way here - this field has to be padded with
> >>> zeroes, so doing "- 1" is wrong here.
> >>
> >> Hi Thomas,
> >>
> >> as I wrote in reply to Phillipe - the array is memset to zeroes before the
> >> if so we
> >> are sure it's padded with zeroes (in this occurrence, not true for second
> >> one).
> > 
> > Ok, but dropping the last character is still wrong here. The ext_names do
> > not need to be terminated with a \0 if they have the full length.
> The current code is actually correct. We are perfectly fine without the final
> \n if the string is really 256 bytes.
> 
> Replacing memset + strncpy with strpadcpy is certainly a good cleanup. Is it
> necessary? No.

Yes, it is necessary because otherwise compiler (GCC 11) produce warning and so
build fail when --enable-werror is used.

Mirek


> 
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Christian Borntraeger]

On 11.01.21 14:17, Miroslav Rezanina wrote:
> 
> 
> ----- Original Message -----
>> From: "Christian Borntraeger" <borntraeger@de.ibm.com>
>> To: "Thomas Huth" <thuth@redhat.com>, "Miroslav Rezanina" <mrezanin@redhat.com>
>> Cc: "qemu-s390x" <qemu-s390x@nongnu.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
>> Sent: Monday, January 11, 2021 2:02:32 PM
>> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
>>
>>
>>
>> On 11.01.21 13:54, Thomas Huth wrote:
>>> On 11/01/2021 13.42, Miroslav Rezanina wrote:
>>>>
>>>>
>>>> ----- Original Message -----
>>>>> From: "Thomas Huth" <thuth@redhat.com>
>>>>> To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, mrezanin@redhat.com,
>>>>> qemu-devel@nongnu.org, "qemu-s390x"
>>>>> <qemu-s390x@nongnu.org>
>>>>> Sent: Monday, January 11, 2021 1:24:57 PM
>>>>> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
>>>>>
>>>>> On 11/01/2021 13.10, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Miroslav,
>>>>>>
>>>>>> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
>>>>>>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>>>>>>
>>>>>>> There are two cases when vm name is copied but closing \0 can be lost
>>>>>>> in case name is too long (>=256 characters).
>>>>>>>
>>>>>>> Updating length to copy so there is space for closing \0.
>>>>>>>
>>>>>>> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
>>>>>>> ---
>>>>>>>    target/s390x/kvm.c         | 2 +-
>>>>>>>    target/s390x/misc_helper.c | 4 +++-
>>>>>>>    2 files changed, 4 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>>>>> index b8385e6b95..2313b5727e 100644
>>>>>>> --- a/target/s390x/kvm.c
>>>>>>> +++ b/target/s390x/kvm.c
>>>>>>> @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
>>>>>>> addr, uint8_t ar)
>>>>>>>         */
>>>>>>>        if (qemu_name) {
>>>>>>>            strncpy((char *)sysib.ext_names[0], qemu_name,
>>>>>>> -                sizeof(sysib.ext_names[0]));
>>>>>>> +                sizeof(sysib.ext_names[0]) - 1);
>>>>>>>        } else {
>>>>>>>            strcpy((char *)sysib.ext_names[0], "KVMguest");
>>>>>>>        }
>>>>>>
>>>>>> What about using strpadcpy() instead?
>>>>>
>>>>> Yes, strpadcpy is the better way here - this field has to be padded with
>>>>> zeroes, so doing "- 1" is wrong here.
>>>>
>>>> Hi Thomas,
>>>>
>>>> as I wrote in reply to Phillipe - the array is memset to zeroes before the
>>>> if so we
>>>> are sure it's padded with zeroes (in this occurrence, not true for second
>>>> one).
>>>
>>> Ok, but dropping the last character is still wrong here. The ext_names do
>>> not need to be terminated with a \0 if they have the full length.
>> The current code is actually correct. We are perfectly fine without the final
>> \n if the string is really 256 bytes.
>>
>> Replacing memset + strncpy with strpadcpy is certainly a good cleanup. Is it
>> necessary? No.
> 
> Yes, it is necessary because otherwise compiler (GCC 11) produce warning and so
> build fail when --enable-werror is used.

Fair enough. But that actually means that the compiler warning is wrong, because
we use strncpy exactly in the way as described (allowing for the final \n to be
missing). But let us use strpadcpy then.

Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length

[Miroslav Rezanina]

----- Original Message -----
> From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
> To: mrezanin@redhat.com, qemu-devel@nongnu.org
> Sent: Monday, January 11, 2021 1:10:07 PM
> Subject: Re: [RHEL7 qemu-kvm PATCH 2/3] s390x: Fix vm name copy length
> 
> Hi Miroslav,
> 
> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> > From: Miroslav Rezanina <mrezanin@redhat.com>
> > 
> > There are two cases when vm name is copied but closing \0 can be lost
> > in case name is too long (>=256 characters).
> > 
> > Updating length to copy so there is space for closing \0.
> > 
> > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
> > ---
> >  target/s390x/kvm.c         | 2 +-
> >  target/s390x/misc_helper.c | 4 +++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index b8385e6b95..2313b5727e 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -1918,7 +1918,7 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64
> > addr, uint8_t ar)
> >       */
> >      if (qemu_name) {
> >          strncpy((char *)sysib.ext_names[0], qemu_name,
> > -                sizeof(sysib.ext_names[0]));
> > +                sizeof(sysib.ext_names[0]) - 1);
> >      } else {
> >          strcpy((char *)sysib.ext_names[0], "KVMguest");
> >      }
> 
> What about using strpadcpy() instead?
> 
>     strpadcpy((char *)sysib.sysib_322.ext_names[0],
>               sizeof(sysib.sysib_322.ext_names[0]),
>               qemu_name ?: "KVMguest", '\0');

Hi Philippe, 

I went with -1 here because code use memset to 0 few lines above this code, so
we now there are only zeroes in the array and we do not have to write them again.

> 
> > diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> > index 58dbc023eb..7c478b9e58 100644
> > --- a/target/s390x/misc_helper.c
> > +++ b/target/s390x/misc_helper.c
> > @@ -369,8 +369,10 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0,
> > uint64_t r0, uint64_t r1)
> >                  ebcdic_put(sysib.sysib_322.vm[0].name, qemu_name,
> >                             MIN(sizeof(sysib.sysib_322.vm[0].name),
> >                                 strlen(qemu_name)));
> > +		memset((char *)sysib.sysib_322.ext_names[0], 0,
> > +		       sizeof(sysib.sysib_322.ext_names[0]));
> >                  strncpy((char *)sysib.sysib_322.ext_names[0], qemu_name,
> > -                        sizeof(sysib.sysib_322.ext_names[0]));
> > +                        sizeof(sysib.sysib_322.ext_names[0]) - 1);
> 
> And here:
> 
>                strpadcpy((char *)sysib.sysib_322.ext_names[0],
>                          sizeof(sysib.sysib_322.ext_names[0]),
>                          qemu_name, '\0');

However, here we are adding memset so using strpadcpy is better choice to
ensure we have \0 after name string.

Mirek

> 
> >              } else {
> >                  ebcdic_put(sysib.sysib_322.vm[0].name, "TCGguest", 8);
> >                  strcpy((char *)sysib.sysib_322.ext_names[0], "TCGguest");
> > 
> 
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

[RHEL7 qemu-kvm PATCH 3/3] Fix tcg_out_op argument mismatch warning

[mrezanin]

From: Miroslav Rezanina <mrezanin@redhat.com>

There's prototype mismatch between tcg/tcg.c and tcg/aarch/tcg-target.c.inc:

tcg.c:

    static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
                           const int *const_args);

tcg-target.c.inc:

    static void tcg_out_op(TCGContext *s, TCGOpcode opc,
                           const TCGArg args[TCG_MAX_OP_ARGS],
                           const int const_args[TCG_MAX_OP_ARGS])

This missmatch cause warnings on GCC 11:

    tcg/aarch64/tcg-target.c.inc:1855:37: error: argument 3 of type 'const TCGArg[16]' {aka 'const long unsigned int[16]'} with mismatched bound [-Werror=array-parameter=]
    tcg/aarch64/tcg-target.c.inc:1856:34: error: argument 4 of type 'const int[16]' with mismatched bound [-Werror=array-parameter=]

Only architectures with this definition are aarch and sparc. Fixing both archs to use
proper argument type.
---
 tcg/aarch64/tcg-target.c.inc | 3 +--
 tcg/sparc/tcg-target.c.inc   | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index 26f71cb599..fe6bdbf721 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -1852,8 +1852,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
 static tcg_insn_unit *tb_ret_addr;
 
 static void tcg_out_op(TCGContext *s, TCGOpcode opc,
-                       const TCGArg args[TCG_MAX_OP_ARGS],
-                       const int const_args[TCG_MAX_OP_ARGS])
+                       const TCGArg *args, const int *const_args)
 {
     /* 99% of the time, we can signal the use of extension registers
        by looking to see if the opcode handles 64-bit data.  */
diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc
index 6775bd30fc..976f0f05af 100644
--- a/tcg/sparc/tcg-target.c.inc
+++ b/tcg/sparc/tcg-target.c.inc
@@ -1294,8 +1294,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data, TCGReg addr,
 }
 
 static void tcg_out_op(TCGContext *s, TCGOpcode opc,
-                       const TCGArg args[TCG_MAX_OP_ARGS],
-                       const int const_args[TCG_MAX_OP_ARGS])
+                       const TCGArg *args, const int *const_args)
 {
     TCGArg a0, a1, a2;
     int c, c2;
-- 
2.18.4

Re: [RHEL7 qemu-kvm PATCH 3/3] Fix tcg_out_op argument mismatch warning

[Philippe Mathieu-Daudé]

On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
> 
> There's prototype mismatch between tcg/tcg.c and tcg/aarch/tcg-target.c.inc:
> 
> tcg.c:
> 
>     static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
>                            const int *const_args);
> 
> tcg-target.c.inc:
> 
>     static void tcg_out_op(TCGContext *s, TCGOpcode opc,
>                            const TCGArg args[TCG_MAX_OP_ARGS],
>                            const int const_args[TCG_MAX_OP_ARGS])
> 
> This missmatch cause warnings on GCC 11:
> 
>     tcg/aarch64/tcg-target.c.inc:1855:37: error: argument 3 of type 'const TCGArg[16]' {aka 'const long unsigned int[16]'} with mismatched bound [-Werror=array-parameter=]
>     tcg/aarch64/tcg-target.c.inc:1856:34: error: argument 4 of type 'const int[16]' with mismatched bound [-Werror=array-parameter=]

TIL. Interesting, compilers are getting smarter :)

> Only architectures with this definition are aarch and sparc. Fixing both archs to use
> proper argument type.
> ---
>  tcg/aarch64/tcg-target.c.inc | 3 +--
>  tcg/sparc/tcg-target.c.inc   | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
> index 26f71cb599..fe6bdbf721 100644
> --- a/tcg/aarch64/tcg-target.c.inc
> +++ b/tcg/aarch64/tcg-target.c.inc
> @@ -1852,8 +1852,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data_reg, TCGReg addr_reg,
>  static tcg_insn_unit *tb_ret_addr;
>  
>  static void tcg_out_op(TCGContext *s, TCGOpcode opc,
> -                       const TCGArg args[TCG_MAX_OP_ARGS],
> -                       const int const_args[TCG_MAX_OP_ARGS])
> +                       const TCGArg *args, const int *const_args)

Doing this way we loose information (that the array pointed has
TCG_MAX_OP_ARGS elements). What about letting this prototype and
fix the other uses?

>  {
>      /* 99% of the time, we can signal the use of extension registers
>         by looking to see if the opcode handles 64-bit data.  */
> diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc
> index 6775bd30fc..976f0f05af 100644
> --- a/tcg/sparc/tcg-target.c.inc
> +++ b/tcg/sparc/tcg-target.c.inc
> @@ -1294,8 +1294,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg data, TCGReg addr,
>  }
>  
>  static void tcg_out_op(TCGContext *s, TCGOpcode opc,
> -                       const TCGArg args[TCG_MAX_OP_ARGS],
> -                       const int const_args[TCG_MAX_OP_ARGS])
> +                       const TCGArg *args, const int *const_args)
>  {
>      TCGArg a0, a1, a2;
>      int c, c2;
> 

Re: [RHEL7 qemu-kvm PATCH 3/3] Fix tcg_out_op argument mismatch warning

[Miroslav Rezanina]

----- Original Message -----
> From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
> To: mrezanin@redhat.com, qemu-devel@nongnu.org
> Cc: "Richard Henderson" <richard.henderson@linaro.org>, "Eric Blake" <eblake@redhat.com>
> Sent: Monday, January 11, 2021 1:15:04 PM
> Subject: Re: [RHEL7 qemu-kvm PATCH 3/3] Fix tcg_out_op argument mismatch warning
> 
> On 1/11/21 12:30 PM, mrezanin@redhat.com wrote:
> > From: Miroslav Rezanina <mrezanin@redhat.com>
> > 
> > There's prototype mismatch between tcg/tcg.c and
> > tcg/aarch/tcg-target.c.inc:
> > 
> > tcg.c:
> > 
> >     static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg
> >     *args,
> >                            const int *const_args);
> > 
> > tcg-target.c.inc:
> > 
> >     static void tcg_out_op(TCGContext *s, TCGOpcode opc,
> >                            const TCGArg args[TCG_MAX_OP_ARGS],
> >                            const int const_args[TCG_MAX_OP_ARGS])
> > 
> > This missmatch cause warnings on GCC 11:
> > 
> >     tcg/aarch64/tcg-target.c.inc:1855:37: error: argument 3 of type 'const
> >     TCGArg[16]' {aka 'const long unsigned int[16]'} with mismatched bound
> >     [-Werror=array-parameter=]
> >     tcg/aarch64/tcg-target.c.inc:1856:34: error: argument 4 of type 'const
> >     int[16]' with mismatched bound [-Werror=array-parameter=]
> 
> TIL. Interesting, compilers are getting smarter :)
> 
> > Only architectures with this definition are aarch and sparc. Fixing both
> > archs to use
> > proper argument type.
> > ---
> >  tcg/aarch64/tcg-target.c.inc | 3 +--
> >  tcg/sparc/tcg-target.c.inc   | 3 +--
> >  2 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
> > index 26f71cb599..fe6bdbf721 100644
> > --- a/tcg/aarch64/tcg-target.c.inc
> > +++ b/tcg/aarch64/tcg-target.c.inc
> > @@ -1852,8 +1852,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg
> > data_reg, TCGReg addr_reg,
> >  static tcg_insn_unit *tb_ret_addr;
> >  
> >  static void tcg_out_op(TCGContext *s, TCGOpcode opc,
> > -                       const TCGArg args[TCG_MAX_OP_ARGS],
> > -                       const int const_args[TCG_MAX_OP_ARGS])
> > +                       const TCGArg *args, const int *const_args)
> 
> Doing this way we loose information (that the array pointed has
> TCG_MAX_OP_ARGS elements). What about letting this prototype and
> fix the other uses?

I'm not author of the code so I went with smaller change - forward definition
in tcg.c and most tct-target.c.inc use this form. I would need someone more
familiar with this part to clarify whether it's ok to go with opposite change.

Mirek

> 
> >  {
> >      /* 99% of the time, we can signal the use of extension registers
> >         by looking to see if the opcode handles 64-bit data.  */
> > diff --git a/tcg/sparc/tcg-target.c.inc b/tcg/sparc/tcg-target.c.inc
> > index 6775bd30fc..976f0f05af 100644
> > --- a/tcg/sparc/tcg-target.c.inc
> > +++ b/tcg/sparc/tcg-target.c.inc
> > @@ -1294,8 +1294,7 @@ static void tcg_out_qemu_st(TCGContext *s, TCGReg
> > data, TCGReg addr,
> >  }
> >  
> >  static void tcg_out_op(TCGContext *s, TCGOpcode opc,
> > -                       const TCGArg args[TCG_MAX_OP_ARGS],
> > -                       const int const_args[TCG_MAX_OP_ARGS])
> > +                       const TCGArg *args, const int *const_args)
> >  {
> >      TCGArg a0, a1, a2;
> >      int c, c2;
> > 
> 
> 

-- 
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer

Re: [RHEL7 qemu-kvm PATCH 0/3] Fixing several GCC 11 warnings

[no-reply]

Patchew URL: https://patchew.org/QEMU/cover.1610364304.git.mrezanin@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: cover.1610364304.git.mrezanin@redhat.com
Subject: [RHEL7 qemu-kvm PATCH 0/3] Fixing several GCC 11 warnings

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/cover.1610364304.git.mrezanin@redhat.com -> patchew/cover.1610364304.git.mrezanin@redhat.com
Switched to a new branch 'test'
c05a0c4 Fix tcg_out_op argument mismatch warning
a8b8ace s390x: Fix vm name copy length
b64e754 Fix net.c warning on GCC 11

=== OUTPUT BEGIN ===
1/3 Checking commit b64e754d193f (Fix net.c warning on GCC 11)
2/3 Checking commit a8b8ace484f8 (s390x: Fix vm name copy length)
ERROR: trailing whitespace
#36: FILE: target/s390x/misc_helper.c:372:
+^I^Imemset((char *)sysib.sysib_322.ext_names[0], 0, $

ERROR: code indent should never use tabs
#36: FILE: target/s390x/misc_helper.c:372:
+^I^Imemset((char *)sysib.sysib_322.ext_names[0], 0, $

ERROR: code indent should never use tabs
#37: FILE: target/s390x/misc_helper.c:373:
+^I^I       sizeof(sysib.sysib_322.ext_names[0]));$

total: 3 errors, 0 warnings, 19 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit c05a0c4ed447 (Fix tcg_out_op argument mismatch warning)
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 18 lines checked

Patch 3/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/cover.1610364304.git.mrezanin@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com