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

[Luis Machado <lgustavo@codesourcery.com>]

[-- 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);
 }