Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-05-06 Thread Daniel Jacobowitz 
On Wed, Apr 27, 2005 at 06:48:46PM +0300, Eli Zaretskii wrote:
 Thanks for the tip.  Here's the revised patch:

The rest of this is mostly stylistic; a few comments...

 +  /* If we don't already know the full absolute file name of the
 + source file, find it now.  */
 +  if (NULL == sal.symtab-fullname)
 +{
 +  fn = symtab_to_fullname (sal.symtab);
 +  if (NULL == fn)
 + fn = unknown;
 +}
 +  else
 +fn = sal.symtab-fullname;
 +
 +  /* $EDITOR  blank  +NN  blankfile  \0 */
 +  cmdlen = strlen(editor) + 1 + log10 + 2 +  strlen(fn) + 1;
p = xmalloc(cmdlen);
 -  sprintf(p,%s +%d %s%s,editor,sal.line,
 - (NULL == sal.symtab-dirname ? ./ :
 -(NULL != sal.symtab-filename  *(sal.symtab-filename) != '/') ?
 -sal.symtab-dirname : ),
 - (NULL == sal.symtab-filename ? unknown : sal.symtab-filename)
 -  );
 +  sprintf (p, %s +%d %s, editor, sal.line, fn);

It strikes me as odd that you can use edit if GDB doesn't know where
the source file is.  I realize this is a pre-existing condition, but...
Also, symtab_to_fullname includes the cached fullname check.  So what's
your opinion of boiling the whole thing down to:

  fn = symtab_to_fullname (sal.symtab);
  if (fn == NULL)
error (_(Could not find file \%s\), sal.symtab-filename);
  p = xstrprintf (%s +%d %s, editor, sal.line, fn);

Mark, can I have those bonus points? :-)

-- 
Daniel Jacobowitz
CodeSourcery, LLC


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb

Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-05-06 Thread Daniel Jacobowitz 
On Thu, Apr 28, 2005 at 11:36:50PM +0300, Eli Zaretskii wrote:
 I ended up committing the attached.  Note that it quotes the file
 name, to account for possible special characters that would confise
 the shell.  (I use ... for quoting because this is more portable to
 various shells, including on Windows.)
 
 Thanks to Daniel and Mark for valuable input.

Fine with me.  Thanks.

The use of  is not a bulletproof quoting mechanism, but it's better
than previously.

-- 
Daniel Jacobowitz
CodeSourcery, LLC


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb

Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-05-06 Thread Daniel Jacobowitz 
On Fri, Apr 29, 2005 at 12:02:27AM +0300, Eli Zaretskii wrote:
  Date: Thu, 28 Apr 2005 16:42:01 -0400
  From: Daniel Jacobowitz [EMAIL PROTECTED]
  Cc: [EMAIL PROTECTED], bug-gdb@gnu.org, [EMAIL PROTECTED]
  
  The use of  is not a bulletproof quoting mechanism
 
 Yep, known.  But it's IMHO the most portable one can get without going
 into the pains of quotesys and its ilk (which doesn't support
 non-Posix shells at all).

Or using a mechanism to start other processes that takes an argument
vector :-)

-- 
Daniel Jacobowitz
CodeSourcery, LLC


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb

Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-05-06 Thread Daniel Jacobowitz 
On Wed, Apr 27, 2005 at 05:30:11PM +0300, Eli Zaretskii wrote:
  Date: Tue, 12 Apr 2005 09:58:25 -0400
  From: [EMAIL PROTECTED]
  
  While using gdb 6.3,  I was having a problem with the directory name and the
  file name being concatenated by the edit command without an intervening 
  slash.
  My solution was just to add the slash to the sprintf format string
  in cli/cli-cmds.c, near line 650.  It worked for me.
 
 Thanks for pointing this out.
 
 In fact, the code there had quite a few problems besides the one you
 found: it didn't use symtab-fullname, failed miserably for DOS-style
 d:/foo/bar file names, etc.
 
 Does anyone object to the following patch?
 
 2005-04-27  Eli Zaretskii  [EMAIL PROTECTED]
 
   * cli/cli-cmds.c (edit_command): Use symtab-fullname if
   possible.  Use IS_ABSOLUTE_PATH instead of checking for a literal
   '/'.  Make sure there's a slash between the directory and the file
   name.  Simplify and clarify the code logic.

I can simplify this a whole lot further :-)

You should use symtab_to_fullname.  Then all the fallback logic is
unnecessary; if symtab_to_fullname fails, GDB does not know where the
file is.

-- 
Daniel Jacobowitz
CodeSourcery, LLC


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb

Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-04-29 Thread Eli Zaretskii 
 Date: Thu, 28 Apr 2005 17:18:03 -0400
 From: Daniel Jacobowitz [EMAIL PROTECTED]
 Cc: bug-gdb@gnu.org, [EMAIL PROTECTED]
 
 Or using a mechanism to start other processes that takes an argument
 vector :-)

Unfortunately, that's not a magic wand, either.  Specifically, in the
case in point, $EDITOR can be a _shell_command_, not a file name of a
program.  That is, I could say

 export EDITOR=emacs -q

and expect it to work.  This will fail with vector-argument method of
invoking subprocesses.


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb

Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-04-28 Thread Eli Zaretskii 
 Date: Wed, 27 Apr 2005 14:04:10 -0400
 From: Daniel Jacobowitz [EMAIL PROTECTED]
 Cc: [EMAIL PROTECTED], bug-gdb@gnu.org, [EMAIL PROTECTED]
 
 It strikes me as odd that you can use edit if GDB doesn't know where
 the source file is.  I realize this is a pre-existing condition, but...
 Also, symtab_to_fullname includes the cached fullname check.  So what's
 your opinion of boiling the whole thing down to:
 
   fn = symtab_to_fullname (sal.symtab);
   if (fn == NULL)
 error (_(Could not find file \%s\), sal.symtab-filename);
   p = xstrprintf (%s +%d %s, editor, sal.line, fn);
 
 Mark, can I have those bonus points? :-)

I ended up committing the attached.  Note that it quotes the file
name, to account for possible special characters that would confise
the shell.  (I use ... for quoting because this is more portable to
various shells, including on Windows.)

Thanks to Daniel and Mark for valuable input.


2005-04-28  Eli Zaretskii  [EMAIL PROTECTED]

* cli/cli-cmds.c (edit_command): If symtab-fullname is not yet
set, use symtab_to_fullname, instead of trying to do its job.  Use
xstrprintf instead of malloc and sprintf.


Index: gdb/cli/cli-cmds.c
===
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.58
retrieving revision 1.59
diff -u -r1.58 -r1.59
--- gdb/cli/cli-cmds.c  16 Mar 2005 15:58:41 -  1.58
+++ gdb/cli/cli-cmds.c  28 Apr 2005 20:32:41 -  1.59
@@ -554,7 +554,7 @@
   int cmdlen, log10;
   unsigned m;
   char *editor;
-  char *p;
+  char *p, *fn;
 
   /* Pull in the current default source line if necessary */
   if (arg == 0)
@@ -627,25 +627,26 @@
 
   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=0x8000; !(sal.line  m)  log100; log10--, m=m1);
   log10 = 1 + (int)((log10 + (0 == ((m-1)  sal.line)))/3.32192809);
 
-  cmdlen = strlen(editor) + 1
- + (NULL == sal.symtab-dirname ? 0 : strlen(sal.symtab-dirname) + 1)
-+ (NULL == sal.symtab-filename? 0 : strlen(sal.symtab-filename)+ 1)
-+ log10 + 2;
-  
-  p = xmalloc(cmdlen);
-  sprintf(p,%s +%d %s%s,editor,sal.line,
- (NULL == sal.symtab-dirname ? ./ :
-(NULL != sal.symtab-filename  *(sal.symtab-filename) != '/') ?
-  sal.symtab-dirname : ),
- (NULL == sal.symtab-filename ? unknown : sal.symtab-filename)
-  );
-  shell_escape(p, from_tty);
+  /* If we don't already know the full absolute file name of the
+ source file, find it now.  */
+  if (!sal.symtab-fullname)
+{
+  fn = symtab_to_fullname (sal.symtab);
+  if (!fn)
+   fn = unknown;
+}
+  else
+fn = sal.symtab-fullname;
 
+  /* 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);
 }
 


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb

Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)

2005-04-27 Thread Eli Zaretskii 
 Date: Wed, 27 Apr 2005 10:36:20 -0400
 From: Daniel Jacobowitz [EMAIL PROTECTED]
 Cc: [EMAIL PROTECTED], bug-gdb@gnu.org, [EMAIL PROTECTED]
 
 I can simplify this a whole lot further :-)
 
 You should use symtab_to_fullname.  Then all the fallback logic is
 unnecessary; if symtab_to_fullname fails, GDB does not know where the
 file is.

Thanks for the tip.  Here's the revised patch:


2005-04-27  Eli Zaretskii  [EMAIL PROTECTED]

* cli/cli-cmds.c (edit_command): If symtab-fullname is not yet
set, use symtab_to_fullname , instead of trying to do its job.

--- gdb/cli/cli-cmds.c~02005-03-26 16:18:14.0 +0300
+++ gdb/cli/cli-cmds.c  2005-04-27 18:37:34.0 +0300
@@ -554,7 +554,7 @@ edit_command (char *arg, int from_tty)
   int cmdlen, log10;
   unsigned m;
   char *editor;
-  char *p;
+  char *p, *fn;
 
   /* Pull in the current default source line if necessary */
   if (arg == 0)
@@ -627,23 +627,26 @@ 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=0x8000; !(sal.line  m)  log100; log10--, m=m1);
   log10 = 1 + (int)((log10 + (0 == ((m-1)  sal.line)))/3.32192809);
 
-  cmdlen = strlen(editor) + 1
- + (NULL == sal.symtab-dirname ? 0 : strlen(sal.symtab-dirname) + 1)
-+ (NULL == sal.symtab-filename? 0 : strlen(sal.symtab-filename)+ 1)
-+ log10 + 2;
-  
+  /* If we don't already know the full absolute file name of the
+ source file, find it now.  */
+  if (NULL == sal.symtab-fullname)
+{
+  fn = symtab_to_fullname (sal.symtab);
+  if (NULL == fn)
+   fn = unknown;
+}
+  else
+fn = sal.symtab-fullname;
+
+  /* $EDITOR  blank  +NN  blankfile  \0 */
+  cmdlen = strlen(editor) + 1 + log10 + 2 +  strlen(fn) + 1;
   p = xmalloc(cmdlen);
-  sprintf(p,%s +%d %s%s,editor,sal.line,
- (NULL == sal.symtab-dirname ? ./ :
-(NULL != sal.symtab-filename  *(sal.symtab-filename) != '/') ?
-  sal.symtab-dirname : ),
- (NULL == sal.symtab-filename ? unknown : sal.symtab-filename)
-  );
+  sprintf (p, %s +%d %s, editor, sal.line, fn);
   shell_escape(p, from_tty);
 
   xfree(p);


___
Bug-gdb mailing list
Bug-gdb@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-gdb