Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RFA: clean up edit_command
@ 2008-07-28 15:50 Tom Tromey
  2008-07-29  2:48 ` Michael Snyder
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2008-07-28 15:50 UTC (permalink / raw)
  To: gdb-patches

I noticed some weird code in edit_command yesterday.

Ordinarily I do not like to mix code changes and formatting fixes, but
in this case, I think it is warranted... the code change here is just
removing a dead loop.  The formatting fixes are the usual: bad brace
style, bad indentation, wrong comment style, spacing missing before
"("... this function had them all :)

Built and regtested on the GCC compile farm (x86-64).

Ok?

Tom

2008-07-28  Tom Tromey  <tromey@redhat.com>

	* cli/cli-cmds.c (edit_command): Remove unused variables.  Delete
	dead code.  Fix indentation.

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 8dc178e..16fbf3d 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -615,12 +615,11 @@ edit_command (char *arg, int from_tty)
   struct symtab_and_line sal;
   struct symbol *sym;
   char *arg1;
-  int cmdlen, log10;
-  unsigned m;
+  int cmdlen;
   char *editor;
   char *p, *fn;
 
-  /* Pull in the current default source line if necessary */
+  /* Pull in the current default source line if necessary.  */
   if (arg == 0)
     {
       set_default_source_symtab_and_line ();
@@ -638,17 +637,22 @@ edit_command (char *arg, int from_tty)
   else
     {
 
-      /* Now should only be one argument -- decode it in SAL */
+      /* Now should only be one argument -- decode it in SAL.  */
 
       arg1 = arg;
       sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
 
-      if (! sals.nelts) return;  /*  C++  */
-      if (sals.nelts > 1) {
-        ambiguous_line_spec (&sals);
-        xfree (sals.sals);
-        return;
-      }
+      if (! sals.nelts)
+	{
+	  /*  C++  */
+	  return;
+	}
+      if (sals.nelts > 1)
+	{
+	  ambiguous_line_spec (&sals);
+	  xfree (sals.sals);
+	  return;
+	}
 
       sal = sals.sals[0];
       xfree (sals.sals);
@@ -656,7 +660,7 @@ edit_command (char *arg, int from_tty)
       if (*arg1)
         error (_("Junk at end of line specification."));
 
-      /* if line was specified by address,
+      /* If line was specified by address,
          first print exactly which line, and which file.
          In this case, sal.symtab == 0 means address is outside
          of all known source files, not that user failed to give a filename.  */
@@ -686,10 +690,6 @@ edit_command (char *arg, int from_tty)
   if ((editor = (char *) getenv ("EDITOR")) == NULL)
       editor = "/bin/ex";
 
-  /* Approximate base-10 log of line to 1 unit for digit count */
-  for(log10=32, m=0x80000000; !(sal.line & m) && log10>0; log10--, m=m>>1);
-  log10 = 1 + (int)((log10 + (0 == ((m-1) & sal.line)))/3.32192809);
-
   /* If we don't already know the full absolute file name of the
      source file, find it now.  */
   if (!sal.symtab->fullname)
@@ -704,8 +704,8 @@ edit_command (char *arg, int from_tty)
   /* Quote the file name, in case it has whitespace or other special
      characters.  */
   p = xstrprintf ("%s +%d \"%s\"", editor, sal.line, fn);
-  shell_escape(p, from_tty);
-  xfree(p);
+  shell_escape (p, from_tty);
+  xfree (p);
 }
 
 static void


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: RFA: clean up edit_command
  2008-07-28 15:50 RFA: clean up edit_command Tom Tromey
@ 2008-07-29  2:48 ` Michael Snyder
  2008-07-29 19:47   ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Snyder @ 2008-07-29  2:48 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Mon, 2008-07-28 at 09:50 -0600, Tom Tromey wrote:
> I noticed some weird code in edit_command yesterday.
> 
> Ordinarily I do not like to mix code changes and formatting fixes, but
> in this case, I think it is warranted... the code change here is just
> removing a dead loop.  The formatting fixes are the usual: bad brace
> style, bad indentation, wrong comment style, spacing missing before
> "("... this function had them all :)
> 
> Built and regtested on the GCC compile farm (x86-64).
> 
> Ok?

OK, but don't you want to remove cmdlen as well?
Doesn't seem to be used any more...

> 
> Tom
> 
> 2008-07-28  Tom Tromey  <tromey@redhat.com>
> 
> 	* cli/cli-cmds.c (edit_command): Remove unused variables.  Delete
> 	dead code.  Fix indentation.
> 
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 8dc178e..16fbf3d 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -615,12 +615,11 @@ edit_command (char *arg, int from_tty)
>    struct symtab_and_line sal;
>    struct symbol *sym;
>    char *arg1;
> -  int cmdlen, log10;
> -  unsigned m;
> +  int cmdlen;
>    char *editor;
>    char *p, *fn;
>  
> -  /* Pull in the current default source line if necessary */
> +  /* Pull in the current default source line if necessary.  */
>    if (arg == 0)
>      {
>        set_default_source_symtab_and_line ();
> @@ -638,17 +637,22 @@ edit_command (char *arg, int from_tty)
>    else
>      {
>  
> -      /* Now should only be one argument -- decode it in SAL */
> +      /* Now should only be one argument -- decode it in SAL.  */
>  
>        arg1 = arg;
>        sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
>  
> -      if (! sals.nelts) return;  /*  C++  */
> -      if (sals.nelts > 1) {
> -        ambiguous_line_spec (&sals);
> -        xfree (sals.sals);
> -        return;
> -      }
> +      if (! sals.nelts)
> +	{
> +	  /*  C++  */
> +	  return;
> +	}
> +      if (sals.nelts > 1)
> +	{
> +	  ambiguous_line_spec (&sals);
> +	  xfree (sals.sals);
> +	  return;
> +	}
>  
>        sal = sals.sals[0];
>        xfree (sals.sals);
> @@ -656,7 +660,7 @@ edit_command (char *arg, int from_tty)
>        if (*arg1)
>          error (_("Junk at end of line specification."));
>  
> -      /* if line was specified by address,
> +      /* If line was specified by address,
>           first print exactly which line, and which file.
>           In this case, sal.symtab == 0 means address is outside
>           of all known source files, not that user failed to give a filename.  */
> @@ -686,10 +690,6 @@ edit_command (char *arg, int from_tty)
>    if ((editor = (char *) getenv ("EDITOR")) == NULL)
>        editor = "/bin/ex";
>  
> -  /* Approximate base-10 log of line to 1 unit for digit count */
> -  for(log10=32, m=0x80000000; !(sal.line & m) && log10>0; log10--, m=m>>1);
> -  log10 = 1 + (int)((log10 + (0 == ((m-1) & sal.line)))/3.32192809);
> -
>    /* If we don't already know the full absolute file name of the
>       source file, find it now.  */
>    if (!sal.symtab->fullname)
> @@ -704,8 +704,8 @@ edit_command (char *arg, int from_tty)
>    /* Quote the file name, in case it has whitespace or other special
>       characters.  */
>    p = xstrprintf ("%s +%d \"%s\"", editor, sal.line, fn);
> -  shell_escape(p, from_tty);
> -  xfree(p);
> +  shell_escape (p, from_tty);
> +  xfree (p);
>  }
>  
>  static void


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: RFA: clean up edit_command
  2008-07-29  2:48 ` Michael Snyder
@ 2008-07-29 19:47   ` Tom Tromey
  2008-07-29 22:00     ` Michael Snyder
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2008-07-29 19:47 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

>>>>> "Michael" == Michael Snyder <msnyder@specifix.com> writes:

Michael> OK, but don't you want to remove cmdlen as well?
Michael> Doesn't seem to be used any more...

Sure.  Here's an updated patch.

Built and regression tested on compile-farm (x86-64).

Ok?

Tom

2008-07-28  Tom Tromey  <tromey@redhat.com>

	* cli/cli-cmds.c (edit_command): Remove unused variables.  Delete
	dead code.  Fix indentation.

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 8dc178e..d9d2c56 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -615,12 +615,10 @@ edit_command (char *arg, int from_tty)
   struct symtab_and_line sal;
   struct symbol *sym;
   char *arg1;
-  int cmdlen, log10;
-  unsigned m;
   char *editor;
   char *p, *fn;
 
-  /* Pull in the current default source line if necessary */
+  /* Pull in the current default source line if necessary.  */
   if (arg == 0)
     {
       set_default_source_symtab_and_line ();
@@ -638,17 +636,22 @@ edit_command (char *arg, int from_tty)
   else
     {
 
-      /* Now should only be one argument -- decode it in SAL */
+      /* Now should only be one argument -- decode it in SAL.  */
 
       arg1 = arg;
       sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
 
-      if (! sals.nelts) return;  /*  C++  */
-      if (sals.nelts > 1) {
-        ambiguous_line_spec (&sals);
-        xfree (sals.sals);
-        return;
-      }
+      if (! sals.nelts)
+	{
+	  /*  C++  */
+	  return;
+	}
+      if (sals.nelts > 1)
+	{
+	  ambiguous_line_spec (&sals);
+	  xfree (sals.sals);
+	  return;
+	}
 
       sal = sals.sals[0];
       xfree (sals.sals);
@@ -656,7 +659,7 @@ edit_command (char *arg, int from_tty)
       if (*arg1)
         error (_("Junk at end of line specification."));
 
-      /* if line was specified by address,
+      /* If line was specified by address,
          first print exactly which line, and which file.
          In this case, sal.symtab == 0 means address is outside
          of all known source files, not that user failed to give a filename.  */
@@ -686,10 +689,6 @@ edit_command (char *arg, int from_tty)
   if ((editor = (char *) getenv ("EDITOR")) == NULL)
       editor = "/bin/ex";
 
-  /* Approximate base-10 log of line to 1 unit for digit count */
-  for(log10=32, m=0x80000000; !(sal.line & m) && log10>0; log10--, m=m>>1);
-  log10 = 1 + (int)((log10 + (0 == ((m-1) & sal.line)))/3.32192809);
-
   /* If we don't already know the full absolute file name of the
      source file, find it now.  */
   if (!sal.symtab->fullname)
@@ -704,8 +703,8 @@ edit_command (char *arg, int from_tty)
   /* Quote the file name, in case it has whitespace or other special
      characters.  */
   p = xstrprintf ("%s +%d \"%s\"", editor, sal.line, fn);
-  shell_escape(p, from_tty);
-  xfree(p);
+  shell_escape (p, from_tty);
+  xfree (p);
 }
 
 static void


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: RFA: clean up edit_command
  2008-07-29 19:47   ` Tom Tromey
@ 2008-07-29 22:00     ` Michael Snyder
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Snyder @ 2008-07-29 22:00 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Tue, 2008-07-29 at 13:47 -0600, Tom Tromey wrote:
> >>>>> "Michael" == Michael Snyder <msnyder@specifix.com> writes:
> 
> Michael> OK, but don't you want to remove cmdlen as well?
> Michael> Doesn't seem to be used any more...
> 
> Sure.  Here's an updated patch.
> 
> Built and regression tested on compile-farm (x86-64).
> 
> Ok?

Looks good.

> 
> Tom
> 
> 2008-07-28  Tom Tromey  <tromey@redhat.com>
> 
> 	* cli/cli-cmds.c (edit_command): Remove unused variables.  Delete
> 	dead code.  Fix indentation.
> 
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 8dc178e..d9d2c56 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -615,12 +615,10 @@ edit_command (char *arg, int from_tty)
>    struct symtab_and_line sal;
>    struct symbol *sym;
>    char *arg1;
> -  int cmdlen, log10;
> -  unsigned m;
>    char *editor;
>    char *p, *fn;
>  
> -  /* Pull in the current default source line if necessary */
> +  /* Pull in the current default source line if necessary.  */
>    if (arg == 0)
>      {
>        set_default_source_symtab_and_line ();
> @@ -638,17 +636,22 @@ edit_command (char *arg, int from_tty)
>    else
>      {
>  
> -      /* Now should only be one argument -- decode it in SAL */
> +      /* Now should only be one argument -- decode it in SAL.  */
>  
>        arg1 = arg;
>        sals = decode_line_1 (&arg1, 0, 0, 0, 0, 0);
>  
> -      if (! sals.nelts) return;  /*  C++  */
> -      if (sals.nelts > 1) {
> -        ambiguous_line_spec (&sals);
> -        xfree (sals.sals);
> -        return;
> -      }
> +      if (! sals.nelts)
> +	{
> +	  /*  C++  */
> +	  return;
> +	}
> +      if (sals.nelts > 1)
> +	{
> +	  ambiguous_line_spec (&sals);
> +	  xfree (sals.sals);
> +	  return;
> +	}
>  
>        sal = sals.sals[0];
>        xfree (sals.sals);
> @@ -656,7 +659,7 @@ edit_command (char *arg, int from_tty)
>        if (*arg1)
>          error (_("Junk at end of line specification."));
>  
> -      /* if line was specified by address,
> +      /* If line was specified by address,
>           first print exactly which line, and which file.
>           In this case, sal.symtab == 0 means address is outside
>           of all known source files, not that user failed to give a filename.  */
> @@ -686,10 +689,6 @@ edit_command (char *arg, int from_tty)
>    if ((editor = (char *) getenv ("EDITOR")) == NULL)
>        editor = "/bin/ex";
>  
> -  /* Approximate base-10 log of line to 1 unit for digit count */
> -  for(log10=32, m=0x80000000; !(sal.line & m) && log10>0; log10--, m=m>>1);
> -  log10 = 1 + (int)((log10 + (0 == ((m-1) & sal.line)))/3.32192809);
> -
>    /* If we don't already know the full absolute file name of the
>       source file, find it now.  */
>    if (!sal.symtab->fullname)
> @@ -704,8 +703,8 @@ edit_command (char *arg, int from_tty)
>    /* Quote the file name, in case it has whitespace or other special
>       characters.  */
>    p = xstrprintf ("%s +%d \"%s\"", editor, sal.line, fn);
> -  shell_escape(p, from_tty);
> -  xfree(p);
> +  shell_escape (p, from_tty);
> +  xfree (p);
>  }
>  
>  static void


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-07-29 22:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-28 15:50 RFA: clean up edit_command Tom Tromey
2008-07-29  2:48 ` Michael Snyder
2008-07-29 19:47   ` Tom Tromey
2008-07-29 22:00     ` Michael Snyder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox