[patch] memory leak, mi-cmd-var

[patch] memory leak, mi-cmd-var

[msnyder]

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

'nother one from Coverity.


[-- Attachment #2: formspec.txt --]
[-- Type: text/plain, Size: 1372 bytes --]

2007-06-28  Michael Snyder  <msnyder@access-company.com>

	* mi/mi-cmd-var.c (mi_cmd_var_set_format): Free strduped memory
	(Coverity).  Also take the opportunity to call error earlier
	(before strdup).

Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.33
diff -p -r1.33 mi-cmd-var.c
*** mi/mi-cmd-var.c	13 Jun 2007 19:08:47 -0000	1.33
--- mi/mi-cmd-var.c	29 Jun 2007 02:52:39 -0000
*************** mi_cmd_var_set_format (char *command, ch
*** 209,218 ****
    if (var == NULL)
      error (_("mi_cmd_var_set_format: Variable object not found"));
  
!   formspec = xstrdup (argv[1]);
!   if (formspec == NULL)
      error (_("mi_cmd_var_set_format: Must specify the format as: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
  
    len = strlen (formspec);
  
    if (strncmp (formspec, "natural", len) == 0)
--- 209,220 ----
    if (var == NULL)
      error (_("mi_cmd_var_set_format: Variable object not found"));
  
!   if (argv[1] == NULL)
      error (_("mi_cmd_var_set_format: Must specify the format as: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
  
+   formspec = xstrdup (argv[1]);
+   make_cleanup (xfree, formspec);
+ 
    len = strlen (formspec);
  
    if (strncmp (formspec, "natural", len) == 0)

Re: [patch] memory leak, mi-cmd-var

[Nick Roberts]

 > 'nother one from Coverity.
 > 
 > 2007-06-28  Michael Snyder  <msnyder@access-company.com>
 > 
 > 	* mi/mi-cmd-var.c (mi_cmd_var_set_format): Free strduped memory
 > 	(Coverity).  Also take the opportunity to call error earlier
 > 	(before strdup).

I don't see why the string has to be duplicated in the first place or how
argv[1] could be NULL.  Why not just:


*** mi-cmd-var.c	14 Jun 2007 10:12:15 +1200	1.33
--- mi-cmd-var.c	29 Jun 2007 15:35:10 +1200	
*************** mi_cmd_var_set_format (char *command, ch
*** 209,217 ****
    if (var == NULL)
      error (_("mi_cmd_var_set_format: Variable object not found"));
  
!   formspec = xstrdup (argv[1]);
!   if (formspec == NULL)
!     error (_("mi_cmd_var_set_format: Must specify the format as: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
  
    len = strlen (formspec);
  
--- 209,215 ----
    if (var == NULL)
      error (_("mi_cmd_var_set_format: Variable object not found"));
  
!   formspec = argv[1];
  
    len = strlen (formspec);
  


 > Index: mi/mi-cmd-var.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
 > retrieving revision 1.33
 > diff -p -r1.33 mi-cmd-var.c
 > *** mi/mi-cmd-var.c	13 Jun 2007 19:08:47 -0000	1.33
 > --- mi/mi-cmd-var.c	29 Jun 2007 02:52:39 -0000
 > *************** mi_cmd_var_set_format (char *command, ch
 > *** 209,218 ****
 >     if (var == NULL)
 >       error (_("mi_cmd_var_set_format: Variable object not found"));
 >   
 > !   formspec = xstrdup (argv[1]);
 > !   if (formspec == NULL)
 >       error (_("mi_cmd_var_set_format: Must specify the format as: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
 >   
 >     len = strlen (formspec);
 >   
 >     if (strncmp (formspec, "natural", len) == 0)
 > --- 209,220 ----
 >     if (var == NULL)
 >       error (_("mi_cmd_var_set_format: Variable object not found"));
 >   
 > !   if (argv[1] == NULL)
 >       error (_("mi_cmd_var_set_format: Must specify the format as: \"natural\", \"binary\", \"decimal\", \"hexadecimal\", or \"octal\""));
 >   
 > +   formspec = xstrdup (argv[1]);
 > +   make_cleanup (xfree, formspec);
 > + 
 >     len = strlen (formspec);
 >   
 >     if (strncmp (formspec, "natural", len) == 0)
-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [patch] memory leak, mi-cmd-var

[msnyder]

 > 'nother one from Coverity.
 >
 > 2007-06-28  Michael Snyder  <msnyder@access-company.com>
 >
 >      * mi/mi-cmd-var.c (mi_cmd_var_set_format): Free strduped memory
 >      (Coverity).  Also take the opportunity to call error earlier
 >      (before strdup).

> I don't see why the string has to be duplicated in the first place or how
> argv[1] could be NULL.  Why not just:

Well... you're right!  At least about the strdup.

About the NULL check -- it was already there, so I don't favor removing
it unles we can demonstrate that it can't be NULL.  But I'll re-submit
after eliminating the strdup.