[PATCH, testsuite] Prevent warnings due to dummy malloc calls.

[PATCH, testsuite] Prevent warnings due to dummy malloc calls.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

Hi,

When running GDB's testsuite with libraries/compilers that are more 
restrictive in terms of warnings, i've found that some tests were 
failing due to malloc being called but not having its return value 
assigned to any variables, leading to this warning:

warning: ignoring return value of 'malloc', declared with attribute 
warn_unused_result [-Wunused-result]

The following patch adjusts those testcases to silence the warnings by 
(1) assigning malloc's return value and (2) freeing that pointer later on.

In the case of gdb.base/randomize.c, we're missing a free call, which 
leads to an unused variable warning.

Ok?

[-- Attachment #2: malloc.diff --]
[-- Type: text/x-patch, Size: 2749 bytes --]

2013-11-26  Luis Machado  <lgustavo@codesourcery.com>

	gdb/testsuite/
	* gdb.base/callfuncs.c (main): Assign malloc's return value
	and free it afterwards.
	* gdb.base/charset-malloc.c (malloc_stub): Likewise.
	* gdb.base/printcmds.c (main): Likewise.
	* gdb.base/randomize.c (main): Free "p".
	* gdb.base/setvar.c (dummy): Assign malloc's return value
	and free it afterwards.

diff --git a/gdb/testsuite/gdb.base/callfuncs.c b/gdb/testsuite/gdb.base/callfuncs.c
index 0d76ee9..3872f5c 100644
--- a/gdb/testsuite/gdb.base/callfuncs.c
+++ b/gdb/testsuite/gdb.base/callfuncs.c
@@ -652,9 +652,13 @@ voidfunc (void)
 
 int main ()
 {
-  malloc(1);
+  void *p = malloc (1);
   t_double_values(double_val1, double_val2);
   t_structs_c(struct_val1);
+
+  if (p != NULL)
+    free (p);
+
   return 0 ;
 }
 
diff --git a/gdb/testsuite/gdb.base/charset-malloc.c b/gdb/testsuite/gdb.base/charset-malloc.c
index 58242a2..54a9bd6 100644
--- a/gdb/testsuite/gdb.base/charset-malloc.c
+++ b/gdb/testsuite/gdb.base/charset-malloc.c
@@ -31,5 +31,8 @@ malloc_stub (void)
 {
   /* charset.exp wants to allocate memory for constants.  So make sure malloc
      gets linked into the program.  */
-  malloc (1);
+  void *p = malloc (1);
+
+  if (p != NULL)
+    free (p);
 }
diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c
index d80c13d..806f421 100644
--- a/gdb/testsuite/gdb.base/printcmds.c
+++ b/gdb/testsuite/gdb.base/printcmds.c
@@ -218,10 +218,13 @@ char invalid_RRR[] = "aaaaaaaaaaaaaaaaaaaa"
 
 int main ()
 {
-  malloc(1);
+  void *p = malloc (1);
 
   /* Prevent AIX linker from removing variables.  */
   return ctable1[0] + ctable2[0] + int1dim[0] + int2dim[0][0]
     + int3dim[0][0][0] + int4dim[0][0][0][0] + teststring[0] +
       *parrays -> array1 + a1[0] + a2[0];
+
+  if (p != NULL)
+    free (p);
 }
diff --git a/gdb/testsuite/gdb.base/randomize.c b/gdb/testsuite/gdb.base/randomize.c
index 6a65663..127a4c7 100644
--- a/gdb/testsuite/gdb.base/randomize.c
+++ b/gdb/testsuite/gdb.base/randomize.c
@@ -24,5 +24,8 @@ int main()
 
   p = malloc (1);
 
+  if (p != NULL)
+    free (p);
+
   return 0; /* print p */
 }
diff --git a/gdb/testsuite/gdb.base/setvar.c b/gdb/testsuite/gdb.base/setvar.c
index 3a80b22..0022dc0 100644
--- a/gdb/testsuite/gdb.base/setvar.c
+++ b/gdb/testsuite/gdb.base/setvar.c
@@ -204,7 +204,7 @@ dummy ()
 {
   /* setvar.exp wants to allocate memory for constants.  So make sure malloc
      gets linked into the program.  */
-  malloc (1);
+  void *p = malloc (1);
 
   /* Some linkers (e.g. on AIX) remove unreferenced variables,
      so make sure to reference them. */
@@ -278,4 +278,7 @@ dummy ()
   sef.field = s1;
   uef.field = u1;
 #endif
+
+  if (p != NULL)
+    free (p);
 }

Re: [PATCH, testsuite] Prevent warnings due to dummy malloc calls.

[Doug Evans]

On Tue, Nov 26, 2013 at 11:19 AM, Luis Machado
<lgustavo@codesourcery.com> wrote:
> Hi,
>
> When running GDB's testsuite with libraries/compilers that are more
> restrictive in terms of warnings, i've found that some tests were failing
> due to malloc being called but not having its return value assigned to any
> variables, leading to this warning:
>
> warning: ignoring return value of 'malloc', declared with attribute
> warn_unused_result [-Wunused-result]
>
> The following patch adjusts those testcases to silence the warnings by (1)
> assigning malloc's return value and (2) freeing that pointer later on.
>
> In the case of gdb.base/randomize.c, we're missing a free call, which leads
> to an unused variable warning.
>
> Ok?

Coping with these kinds of warnings is a really slippery slope, the
testsuite is just not ready IMO.
OTOH, I don't mind changes like this particular one.

One nit:  Please change all occurrences of this:

+  if (p != NULL)
+    free (p);

to this

+  free (p);

It's simpler and equally correct.

The randomize.c case is a bit odd, printing p after it's freed.
Maybe just add a comment explaining why things are the way they are?

diff --git a/gdb/testsuite/gdb.base/randomize.c
b/gdb/testsuite/gdb.base/randomize.c
index 6a65663..127a4c7 100644
--- a/gdb/testsuite/gdb.base/randomize.c
+++ b/gdb/testsuite/gdb.base/randomize.c
@@ -24,5 +24,8 @@ int main()
   p = malloc (1);

+  if (p != NULL)
+    free (p);
+
   return 0; /* print p */
 }

Re: [PATCH, testsuite] Prevent warnings due to dummy malloc calls.

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

On 11/26/2013 07:12 PM, Doug Evans wrote:
> On Tue, Nov 26, 2013 at 11:19 AM, Luis Machado
> <lgustavo@codesourcery.com> wrote:
>> Hi,
>>
>> When running GDB's testsuite with libraries/compilers that are more
>> restrictive in terms of warnings, i've found that some tests were failing
>> due to malloc being called but not having its return value assigned to any
>> variables, leading to this warning:
>>
>> warning: ignoring return value of 'malloc', declared with attribute
>> warn_unused_result [-Wunused-result]
>>
>> The following patch adjusts those testcases to silence the warnings by (1)
>> assigning malloc's return value and (2) freeing that pointer later on.
>>
>> In the case of gdb.base/randomize.c, we're missing a free call, which leads
>> to an unused variable warning.
>>
>> Ok?
>
> Coping with these kinds of warnings is a really slippery slope, the
> testsuite is just not ready IMO.

Agreed. I had the greatest idea of checking other warnings in the 
testsuite... but i came back to my senses and dropped it for now. :-)

> OTOH, I don't mind changes like this particular one.
>
> One nit:  Please change all occurrences of this:
>
> +  if (p != NULL)
> +    free (p);
>
> to this
>
> +  free (p);
>
> It's simpler and equally correct.

Done!

>
> The randomize.c case is a bit odd, printing p after it's freed.
> Maybe just add a comment explaining why things are the way they are?
>
> diff --git a/gdb/testsuite/gdb.base/randomize.c
> b/gdb/testsuite/gdb.base/randomize.c
> index 6a65663..127a4c7 100644
> --- a/gdb/testsuite/gdb.base/randomize.c
> +++ b/gdb/testsuite/gdb.base/randomize.c
> @@ -24,5 +24,8 @@ int main()
>     p = malloc (1);
>
> +  if (p != NULL)
> +    free (p);
> +
>     return 0; /* print p */
>   }
>

Yeah, this really needs a different change, as done in the attached patch.

Thanks!
Luis

[-- Attachment #2: malloc_v2.diff --]
[-- Type: text/x-patch, Size: 2664 bytes --]

2013-11-26  Luis Machado  <lgustavo@codesourcery.com>

	gdb/testsuite/
	* gdb.base/callfuncs.c (main): Assign malloc's return value
	and free it afterwards.
	* gdb.base/charset-malloc.c (malloc_stub): Likewise.
	* gdb.base/printcmds.c (main): Likewise.
	* gdb.base/randomize.c (main): Free "p".
	* gdb.base/setvar.c (dummy): Assign malloc's return value
	and free it afterwards.

diff --git a/gdb/testsuite/gdb.base/callfuncs.c b/gdb/testsuite/gdb.base/callfuncs.c
index 0d76ee9..c645e0a 100644
--- a/gdb/testsuite/gdb.base/callfuncs.c
+++ b/gdb/testsuite/gdb.base/callfuncs.c
@@ -652,9 +652,10 @@ voidfunc (void)
 
 int main ()
 {
-  malloc(1);
+  void *p = malloc (1);
   t_double_values(double_val1, double_val2);
   t_structs_c(struct_val1);
+  free (p);
   return 0 ;
 }
 
diff --git a/gdb/testsuite/gdb.base/charset-malloc.c b/gdb/testsuite/gdb.base/charset-malloc.c
index 58242a2..565f872 100644
--- a/gdb/testsuite/gdb.base/charset-malloc.c
+++ b/gdb/testsuite/gdb.base/charset-malloc.c
@@ -31,5 +31,6 @@ malloc_stub (void)
 {
   /* charset.exp wants to allocate memory for constants.  So make sure malloc
      gets linked into the program.  */
-  malloc (1);
+  void *p = malloc (1);
+  free (p);
 }
diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c
index d80c13d..57e04e6 100644
--- a/gdb/testsuite/gdb.base/printcmds.c
+++ b/gdb/testsuite/gdb.base/printcmds.c
@@ -218,10 +218,11 @@ char invalid_RRR[] = "aaaaaaaaaaaaaaaaaaaa"
 
 int main ()
 {
-  malloc(1);
+  void *p = malloc (1);
 
   /* Prevent AIX linker from removing variables.  */
   return ctable1[0] + ctable2[0] + int1dim[0] + int2dim[0][0]
     + int3dim[0][0][0] + int4dim[0][0][0][0] + teststring[0] +
       *parrays -> array1 + a1[0] + a2[0];
+  free (p);
 }
diff --git a/gdb/testsuite/gdb.base/randomize.c b/gdb/testsuite/gdb.base/randomize.c
index 6a65663..4c91626 100644
--- a/gdb/testsuite/gdb.base/randomize.c
+++ b/gdb/testsuite/gdb.base/randomize.c
@@ -24,5 +24,6 @@ int main()
 
   p = malloc (1);
 
-  return 0; /* print p */
+  free (p); /* print p */
+  return 0;
 }
diff --git a/gdb/testsuite/gdb.base/setvar.c b/gdb/testsuite/gdb.base/setvar.c
index 3a80b22..5d08602 100644
--- a/gdb/testsuite/gdb.base/setvar.c
+++ b/gdb/testsuite/gdb.base/setvar.c
@@ -204,7 +204,7 @@ dummy ()
 {
   /* setvar.exp wants to allocate memory for constants.  So make sure malloc
      gets linked into the program.  */
-  malloc (1);
+  void *p = malloc (1);
 
   /* Some linkers (e.g. on AIX) remove unreferenced variables,
      so make sure to reference them. */
@@ -278,4 +278,5 @@ dummy ()
   sef.field = s1;
   uef.field = u1;
 #endif
+  free (p);
 }

Re: [PATCH, testsuite] Prevent warnings due to dummy malloc calls.

[Luis Machado]

On 11/26/2013 07:31 PM, Luis Machado wrote:
> On 11/26/2013 07:12 PM, Doug Evans wrote:
>> On Tue, Nov 26, 2013 at 11:19 AM, Luis Machado
>> <lgustavo@codesourcery.com> wrote:
>>> Hi,
>>>
>>> When running GDB's testsuite with libraries/compilers that are more
>>> restrictive in terms of warnings, i've found that some tests were
>>> failing
>>> due to malloc being called but not having its return value assigned
>>> to any
>>> variables, leading to this warning:
>>>
>>> warning: ignoring return value of 'malloc', declared with attribute
>>> warn_unused_result [-Wunused-result]
>>>
>>> The following patch adjusts those testcases to silence the warnings
>>> by (1)
>>> assigning malloc's return value and (2) freeing that pointer later on.
>>>
>>> In the case of gdb.base/randomize.c, we're missing a free call, which
>>> leads
>>> to an unused variable warning.
>>>
>>> Ok?
>>
>> Coping with these kinds of warnings is a really slippery slope, the
>> testsuite is just not ready IMO.
>
> Agreed. I had the greatest idea of checking other warnings in the
> testsuite... but i came back to my senses and dropped it for now. :-)
>
>> OTOH, I don't mind changes like this particular one.
>>
>> One nit:  Please change all occurrences of this:
>>
>> +  if (p != NULL)
>> +    free (p);
>>
>> to this
>>
>> +  free (p);
>>
>> It's simpler and equally correct.
>
> Done!
>
>>
>> The randomize.c case is a bit odd, printing p after it's freed.
>> Maybe just add a comment explaining why things are the way they are?
>>
>> diff --git a/gdb/testsuite/gdb.base/randomize.c
>> b/gdb/testsuite/gdb.base/randomize.c
>> index 6a65663..127a4c7 100644
>> --- a/gdb/testsuite/gdb.base/randomize.c
>> +++ b/gdb/testsuite/gdb.base/randomize.c
>> @@ -24,5 +24,8 @@ int main()
>>     p = malloc (1);
>>
>> +  if (p != NULL)
>> +    free (p);
>> +
>>     return 0; /* print p */
>>   }
>>
>
> Yeah, this really needs a different change, as done in the attached patch.
>
> Thanks!
> Luis

I've checked this in now.