[RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)
> 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.
--- gdb/cli/cli-cmds.c~02005-03-26 16:18:14.0 +0300
+++ gdb/cli/cli-cmds.c 2005-04-27 17:14:46.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, *dn;
/* Pull in the current default source line if necessary */
if (arg == 0)
@@ -627,23 +627,33 @@ 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) && log10>0; log10--, m=m>>1);
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;
-
+ dn = "";
+ if (NULL != sal.symtab->fullname)
+fn = sal.symtab->fullname;
+ else if (NULL == sal.symtab->filename)
+{
+ fn = "unknown";
+ if (NULL != sal.symtab->dirname)
+ dn = sal.symtab->dirname;
+}
+ else if (IS_ABSOLUTE_PATH (sal.symtab->filename))
+fn = sal.symtab->filename;
+ else
+{
+ fn = sal.symtab->filename;
+ dn = sal.symtab->dirname;
+ if (NULL == dn)
+ dn = ".";
+}
+ /* $EDITOR blank +NN blankdir / file \0 */
+ cmdlen = strlen(editor) + 1 + log10 + 2 + strlen(dn) + 1 + 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%s%s", editor, sal.line, dn, (*dn ? "/" : ""), 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)
> 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) && log10>0; log10--, m=m>>1);
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
Re: [RFA] Fix file name generation in edit_command (was: Ver 6.3 edit command failing)
> 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 thought about that, and decided to leave that code alone: GDB might
indeed not know where's the source, but the user could know more.
Most editors let you open files from inside the editor, so a user
could still edit the file.
> 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? :-)
Well, perhaps you should commit this, then. There's not a single line
written by me in this snippet, so it would be misleading for me to
claim the credits in the logs.
___
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)
> 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) && log10>0; log10--, m=m>>1);
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)
> 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). ___ 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)
> 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)
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)
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)
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)
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
