[PATCH] tst-mallinfo2.c: Use correct multiple for total variable

[PATCH] tst-mallinfo2.c: Use correct multiple for total variable

[Yang Xu]

Since test uses 160 multiple for malloc size, we should also use 160 multiple
for total variable instead of 16, then comparison is meaningful. So fix it.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 malloc/tst-mallinfo2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c
index 59a15cf7a8..a2a2550931 100644
--- a/malloc/tst-mallinfo2.c
+++ b/malloc/tst-mallinfo2.c
@@ -68,7 +68,7 @@ do_test (void)
   for (i = 1; i < 20; ++i)
     {
       ptr = malloc (160 * i);
-      total += 16 * i;
+      total += 160 * i;
     }
 
   mi2 = mallinfo2 ();
-- 
2.23.0

Re: [PATCH] tst-mallinfo2.c: Use correct multiple for total variable

[DJ Delorie]

Yang Xu <xuyang2018.jy@cn.fujitsu.com> writes:
> Since test uses 160 multiple for malloc size, we should also use 160 multiple
> for total variable instead of 16, then comparison is meaningful. So fix it.

Wouldn't we need to change the ">" to ">=" so that the test is
technically valid?

> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

FYI glibc doesn't use the signed-off-by tag; we actually require
copyright assignment for large patches (this isn't large enough :).

>  malloc/tst-mallinfo2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c
> index 59a15cf7a8..a2a2550931 100644
> --- a/malloc/tst-mallinfo2.c
> +++ b/malloc/tst-mallinfo2.c
> @@ -68,7 +68,7 @@ do_test (void)
>    for (i = 1; i < 20; ++i)
>      {
>        ptr = malloc (160 * i);
> -      total += 16 * i;
> +      total += 160 * i;
>      }
>  
>    mi2 = mallinfo2 ();

Re: [PATCH] tst-mallinfo2.c: Use correct multiple for total variable

[Yang Xu]

Hi Delorie
> Yang Xu<xuyang2018.jy@cn.fujitsu.com>  writes:
>> Since test uses 160 multiple for malloc size, we should also use 160 multiple
>> for total variable instead of 16, then comparison is meaningful. So fix it.
> 
> Wouldn't we need to change the ">" to">=" so that the test is
> technically valid?

Do you mean "TEST_VERIFY (mi2.uordblks > mi1.uordblks + total);"?
From my test result, this value is greater than "mi1.uordblks + total" .
I think using ">" is ok.
> 
>> Signed-off-by: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
> 
> FYI glibc doesn't use the signed-off-by tag; we actually require
> copyright assignment for large patches (this isn't large enough :).
OK. I understand.
> 
>>   malloc/tst-mallinfo2.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c
>> index 59a15cf7a8..a2a2550931 100644
>> --- a/malloc/tst-mallinfo2.c
>> +++ b/malloc/tst-mallinfo2.c
>> @@ -68,7 +68,7 @@ do_test (void)
>>     for (i = 1; i<  20; ++i)
>>       {
>>         ptr = malloc (160 * i);
>> -      total += 16 * i;
>> +      total += 160 * i;
>>       }
>>
>>     mi2 = mallinfo2 ();
> 
> 
> 
> .
> 

Re: [PATCH] tst-mallinfo2.c: Use correct multiple for total variable

[DJ Delorie]

Yang Xu <xuyang2018.jy@cn.fujitsu.com> writes:
>> Wouldn't we need to change the ">" to">=" so that the test is
>> technically valid?
>
> Do you mean "TEST_VERIFY (mi2.uordblks > mi1.uordblks + total);"?

Yes.

> From my test result, this value is greater than "mi1.uordblks + total" .
> I think using ">" is ok.

There's a huge difference between "correct" and "happens to work".  We
add 160*20 bytes to the pool, we should test that 160*20 or more bytes
are recorded.

Re: [PATCH] tst-mallinfo2.c: Use correct multiple for total variable

[Yang Xu]

on 2021/2/16 11:44, DJ Delorie wrote:
> Yang Xu<xuyang2018.jy@cn.fujitsu.com>  writes:
>>> Wouldn't we need to change the ">" to">=" so that the test is
>>> technically valid?
>>
>> Do you mean "TEST_VERIFY (mi2.uordblks>  mi1.uordblks + total);"?
> 
> Yes.
> 
>>  From my test result, this value is greater than "mi1.uordblks + total" .
>> I think using ">" is ok.
> 
> There's a huge difference between "correct" and "happens to work".  We
> add 160*20 bytes to the pool, we should test that 160*20 or more bytes
> are recorded.
Sound reasonable. I will send v2.
> 
> 
> 
> .
> 

[PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable

[Yang Xu]

Since test uses 160 multiple for malloc size, we should also use 160 multiple
for total variable instead of 16, then comparison is meaningful. So fix it.

Also change the ">" to ">=" so that the test is technically valid.
---
 malloc/tst-mallinfo2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c
index 59a15cf7a8..7d00b65f6e 100644
--- a/malloc/tst-mallinfo2.c
+++ b/malloc/tst-mallinfo2.c
@@ -68,14 +68,14 @@ do_test (void)
   for (i = 1; i < 20; ++i)
     {
       ptr = malloc (160 * i);
-      total += 16 * i;
+      total += 160 * i;
     }
 
   mi2 = mallinfo2 ();
   print_mi ("after", &mi2);
 
   /* Check at least something changed.  */
-  TEST_VERIFY (mi2.uordblks > mi1.uordblks + total);
+  TEST_VERIFY (mi2.uordblks >= mi1.uordblks + total);
 
   return 0;
 }
-- 
2.23.0

Re: [PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable

[Carlos O'Donell]

On 2/17/21 9:56 PM, Yang Xu wrote:
> Since test uses 160 multiple for malloc size, we should also use 160 multiple
> for total variable instead of 16, then comparison is meaningful. So fix it.
 
> Also change the ">" to ">=" so that the test is technically valid.

Yang,

Thank you very much for your great work with this patch.

I need your help in order to complete the FSF copyright assignment process
with Fujitsu.

I've sent you an Naohiro and email to start the process.

Thank you for your help.

> ---
>  malloc/tst-mallinfo2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c
> index 59a15cf7a8..7d00b65f6e 100644
> --- a/malloc/tst-mallinfo2.c
> +++ b/malloc/tst-mallinfo2.c
> @@ -68,14 +68,14 @@ do_test (void)
>    for (i = 1; i < 20; ++i)
>      {
>        ptr = malloc (160 * i);
> -      total += 16 * i;
> +      total += 160 * i;
>      }
>  
>    mi2 = mallinfo2 ();
>    print_mi ("after", &mi2);
>  
>    /* Check at least something changed.  */
> -  TEST_VERIFY (mi2.uordblks > mi1.uordblks + total);
> +  TEST_VERIFY (mi2.uordblks >= mi1.uordblks + total);
>  
>    return 0;
>  }
> 


-- 
Cheers,
Carlos.

RE: [PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable

[xuyang2018.jy]

Hi Carlos , DJ

Fujitsu submitted signed assignment finally.
So please kindly merge the following patch.

https://patchwork.ozlabs.org/project/glibc/patch/1613616993-2623-1-git-send-email-xuyang2018.jy@cn.fujitsu.com/

Best Regards
Yang Xu
-----Original Message-----
From: Carlos O'Donell <carlos@redhat.com> 
Sent: Tuesday, February 23, 2021 10:30 AM
To: Xu, Yang/徐 杨 <xuyang2018.jy@cn.fujitsu.com>; libc-alpha@sourceware.org; DJ Delorie <dj@redhat.com>
Subject: Re: [PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable

On 2/17/21 9:56 PM, Yang Xu wrote:
> Since test uses 160 multiple for malloc size, we should also use 160 
> multiple for total variable instead of 16, then comparison is meaningful. So fix it.
 
> Also change the ">" to ">=" so that the test is technically valid.

Yang,

Thank you very much for your great work with this patch.

I need your help in order to complete the FSF copyright assignment process with Fujitsu.

I've sent you an Naohiro and email to start the process.

Thank you for your help.

> ---
>  malloc/tst-mallinfo2.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c index 
> 59a15cf7a8..7d00b65f6e 100644
> --- a/malloc/tst-mallinfo2.c
> +++ b/malloc/tst-mallinfo2.c
> @@ -68,14 +68,14 @@ do_test (void)
>    for (i = 1; i < 20; ++i)
>      {
>        ptr = malloc (160 * i);
> -      total += 16 * i;
> +      total += 160 * i;
>      }
>  
>    mi2 = mallinfo2 ();
>    print_mi ("after", &mi2);
>  
>    /* Check at least something changed.  */
> -  TEST_VERIFY (mi2.uordblks > mi1.uordblks + total);
> +  TEST_VERIFY (mi2.uordblks >= mi1.uordblks + total);
>  
>    return 0;
>  }
> 


--
Cheers,
Carlos.

RE: [PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable

[naohirot]

Hi Carlos , DJ,
cc: Yang,

> From: Libc-alpha <libc-alpha-bounces@sourceware.org> On Behalf Of
> xuyang2018.jy@fujitsu.com
> Hi Carlos , DJ
> 
> Fujitsu submitted signed assignment finally.
> So please kindly merge the following patch.
> 
> https://patchwork.ozlabs.org/project/glibc/patch/1613616993-2623-1-git-send
> -email-xuyang2018.jy@cn.fujitsu.com/

Fujitsu has received FSF signed assignment.
So the contract process has completed.

Thanks.
Naohiro
 
> Best Regards
> Yang Xu
> -----Original Message-----
> From: Carlos O'Donell <carlos@redhat.com>
> Sent: Tuesday, February 23, 2021 10:30 AM
> To: Xu, Yang/徐 杨 <xuyang2018.jy@cn.fujitsu.com>;
> libc-alpha@sourceware.org; DJ Delorie <dj@redhat.com>
> Subject: Re: [PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable
> 
> On 2/17/21 9:56 PM, Yang Xu wrote:
> > Since test uses 160 multiple for malloc size, we should also use 160
> > multiple for total variable instead of 16, then comparison is meaningful. So fix it.
> 
> > Also change the ">" to ">=" so that the test is technically valid.
> 
> Yang,
> 
> Thank you very much for your great work with this patch.
> 
> I need your help in order to complete the FSF copyright assignment process with
> Fujitsu.
> 
> I've sent you an Naohiro and email to start the process.
> 
> Thank you for your help.
> 
> > ---
> >  malloc/tst-mallinfo2.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/malloc/tst-mallinfo2.c b/malloc/tst-mallinfo2.c index
> > 59a15cf7a8..7d00b65f6e 100644
> > --- a/malloc/tst-mallinfo2.c
> > +++ b/malloc/tst-mallinfo2.c
> > @@ -68,14 +68,14 @@ do_test (void)
> >    for (i = 1; i < 20; ++i)
> >      {
> >        ptr = malloc (160 * i);
> > -      total += 16 * i;
> > +      total += 160 * i;
> >      }
> >
> >    mi2 = mallinfo2 ();
> >    print_mi ("after", &mi2);
> >
> >    /* Check at least something changed.  */
> > -  TEST_VERIFY (mi2.uordblks > mi1.uordblks + total);
> > +  TEST_VERIFY (mi2.uordblks >= mi1.uordblks + total);
> >
> >    return 0;
> >  }
> >
> 
> 
> --
> Cheers,
> Carlos.
> 
> 

Re: [PATCH v2] tst-mallinfo2.c: Use correct multiple for total variable

[DJ Delorie]

"naohirot@fujitsu.com" <naohirot@fujitsu.com> writes:
>> Fujitsu submitted signed assignment finally.
>> So please kindly merge the following patch.

I see the assignment on our side, so I've committed your patch.  Thank
you for your patience :-)