Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Re: [RFC] Let "gcore" command accept a suffix argument
       [not found]               ` <daef60380912091827u6ff7127bp1d03998a40932914@mail.gmail.com>
@ 2009-12-10  2:32                 ` Hui Zhu
  2009-12-10 17:08                   ` Tom Tromey
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2009-12-10  2:32 UTC (permalink / raw)
  To: Michael Snyder, Joel Brobecker; +Cc: gdb-patches ml

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

Sorry I forgot post it to gdb-patches.

Hi guys,

I make a patch to make gdb support "eval gcore foo.$a" command.

(gdb) set $cmd="pwd"
(gdb) eval $cmd
Working directory /home/teawater/gdb/bgdbno/gdb.
(gdb) set $cmd="file ~/gdb/a.out"
(gdb) eval $cmd
Reading symbols from /home/teawater/gdb/a.out...done.
(gdb) start
Temporary breakpoint 1 at 0x80483be
Starting program: /home/teawater/gdb/a.out

Temporary breakpoint 1, 0x080483be in main ()
(gdb) set $name="eval"
(gdb) set $num=3
(gdb) eval gcore $name.$num
Saved corefile eval.3

Please post your comments about it.

And the help and file that include this command is not very well.
Please help me with them.  :)

Thanks,
Hui


2009-12-10  Hui Zhu  <teawater@gmail.com>
       * printcmd.c (ctype.h): New include.
       (eval_command): New function.
       (_initialize_printcmd): New command "eval".

---
 printcmd.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

--- a/printcmd.c
+++ b/printcmd.c
@@ -50,6 +50,8 @@
 #include "parser-defs.h"
 #include "charset.h"

+#include <ctype.h>
+
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et.al.   */
 #endif
@@ -2625,6 +2627,84 @@ printf_command (char *arg, int from_tty)
   do_cleanups (old_cleanups);
 }

+static void
+eval_command (char *exp, int from_tty)
+{
+#define CMDSIZE 1024
+  char cmd[CMDSIZE + 1];
+  char *cmdp = cmd;
+  int is_eval = 0;
+  char *eval_begin;
+
+  if (!exp)
+    return;
+
+  while (cmdp - cmd < CMDSIZE)
+    {
+      if (is_eval)
+        {
+          if (!isalnum (exp[0]) && exp[0] != '_')
+            {
+              struct value *value;
+              gdb_byte *buffer;
+              int length = -1;
+              struct type *char_type = NULL;
+              const char *la_encoding = NULL;
+              char tmp = exp[0];
+
+              exp[0] = '\0';
+              value = parse_and_eval (eval_begin);
+
+              switch (TYPE_CODE (value_type (value)))
+                {
+                  case TYPE_CODE_ARRAY:
+                    LA_GET_STRING (value, &buffer, &length,
+                                   &char_type, &la_encoding);
+                   break;
+                  case TYPE_CODE_INT:
+                    buffer = plongest (value_as_long (value));
+                    length = strlen (buffer);
+                    break;
+                  default:
+                    buffer = eval_begin;
+                    length = exp - eval_begin;
+                    break;
+                }
+
+              if (length > CMDSIZE - (cmdp - cmd))
+                length = CMDSIZE - (cmdp - cmd);
+              memcpy (cmdp, buffer, length);
+              cmdp += length;
+
+              exp[0] = tmp;
+              is_eval = 0;
+            }
+          else
+            exp ++;
+        }
+      else
+        {
+          if (!exp[0])
+            break;
+
+          if (exp[0] == '$')
+            {
+              is_eval = 1;
+              eval_begin = exp;
+            }
+          else
+            {
+              cmdp[0] = exp[0];
+              cmdp ++;
+            }
+          exp ++;
+        }
+    }
+  cmdp[0] = '\0';
+
+  execute_command (cmd, from_tty);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2788,4 +2868,7 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Call command with variable."));
 }

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

---
 printcmd.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

--- a/printcmd.c
+++ b/printcmd.c
@@ -50,6 +50,8 @@
 #include "parser-defs.h"
 #include "charset.h"
 
+#include <ctype.h>
+
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et.al.   */
 #endif
@@ -2625,6 +2627,84 @@ printf_command (char *arg, int from_tty)
   do_cleanups (old_cleanups);
 }
 
+static void
+eval_command (char *exp, int from_tty)
+{
+#define CMDSIZE 1024
+  char cmd[CMDSIZE + 1];
+  char *cmdp = cmd;
+  int is_eval = 0;
+  char *eval_begin;
+
+  if (!exp)
+    return;
+
+  while (cmdp - cmd < CMDSIZE)
+    {
+      if (is_eval)
+        {
+          if (!isalnum (exp[0]) && exp[0] != '_')
+            {
+              struct value *value;
+              gdb_byte *buffer;
+              int length = -1;
+              struct type *char_type = NULL;
+              const char *la_encoding = NULL;
+              char tmp = exp[0];
+
+              exp[0] = '\0';
+              value = parse_and_eval (eval_begin);
+
+              switch (TYPE_CODE (value_type (value)))
+                {
+                  case TYPE_CODE_ARRAY:
+                    LA_GET_STRING (value, &buffer, &length,
+                                   &char_type, &la_encoding);
+                   break;
+                  case TYPE_CODE_INT:
+                    buffer = plongest (value_as_long (value));
+                    length = strlen (buffer);
+                    break;
+                  default:
+                    buffer = eval_begin;
+                    length = exp - eval_begin;
+                    break;
+                }
+
+              if (length > CMDSIZE - (cmdp - cmd))
+                length = CMDSIZE - (cmdp - cmd);
+              memcpy (cmdp, buffer, length);
+              cmdp += length;
+
+              exp[0] = tmp;
+              is_eval = 0;
+            }
+          else
+            exp ++;
+        }
+      else
+        {
+          if (!exp[0])
+            break;
+
+          if (exp[0] == '$')
+            {
+              is_eval = 1;
+              eval_begin = exp;
+            }
+          else
+            {
+              cmdp[0] = exp[0];
+              cmdp ++;
+            }
+          exp ++;
+        }
+    }
+  cmdp[0] = '\0';
+
+  execute_command (cmd, from_tty);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2788,4 +2868,7 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Call command with variable."));
 }

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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-10  2:32                 ` [RFC] Let "gcore" command accept a suffix argument Hui Zhu
@ 2009-12-10 17:08                   ` Tom Tromey
  2009-12-11 10:06                     ` Joel Brobecker
  0 siblings, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2009-12-10 17:08 UTC (permalink / raw)
  To: Hui Zhu; +Cc: Michael Snyder, Joel Brobecker, gdb-patches ml

>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:

>> I make a patch to make gdb support "eval gcore foo.$a" command.

Thanks!

>> +{
>> +#define CMDSIZE 1024
>> +  char cmd[CMDSIZE + 1];

Don't use a fixed-size buffer.  Allocate on the heap and use a cleanup.

>> +              value = parse_and_eval (eval_begin);

This is going to give an error if anything appears after "$foo".
Try something like:

  eval echo $foo $bar

You have multiple choices here.

You could make it only support convenience variables.  Then you would
just have to extract the name and then look up the value.

Or you could try to introduce syntax allowing arbitrary expressions.
One idea that comes to mind is to reuse some code and make eval work
like printf:

  eval "echo %s %d", $foo, $bar + 87

Now that I write it that seems weird :-)

>> +  add_com ("eval", no_class, eval_command, _("\
>> +Call command with variable."));

Exactly what to write depends on how it is implemented.

This also needs a documentation change, a NEWS entry, and test cases.

Tom


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-10 17:08                   ` Tom Tromey
@ 2009-12-11 10:06                     ` Joel Brobecker
  2009-12-11 18:25                       ` Tom Tromey
  0 siblings, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2009-12-11 10:06 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Hui Zhu, Michael Snyder, gdb-patches ml

> Or you could try to introduce syntax allowing arbitrary expressions.
> One idea that comes to mind is to reuse some code and make eval work
> like printf:
> 
>   eval "echo %s %d", $foo, $bar + 87
> 
> Now that I write it that seems weird :-)

I don't think it seems weird. I find this pretty nice, and since we
already have printf, I wonder how hard it would be to implement this?

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-11 10:06                     ` Joel Brobecker
@ 2009-12-11 18:25                       ` Tom Tromey
  2009-12-12  8:33                         ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2009-12-11 18:25 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Hui Zhu, Michael Snyder, gdb-patches ml

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> I don't think it seems weird. I find this pretty nice, and since
Joel> we already have printf, I wonder how hard it would be to implement
Joel> this?

Easy :-)

Tom


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-11 18:25                       ` Tom Tromey
@ 2009-12-12  8:33                         ` Hui Zhu
  2009-12-13  4:05                           ` Hui Zhu
  2009-12-14 17:09                           ` Tom Tromey
  0 siblings, 2 replies; 39+ messages in thread
From: Hui Zhu @ 2009-12-12  8:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, Michael Snyder, gdb-patches ml

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

Hi guys,

I change it to:
eval echo "++$a"

Thanks,
Hui

On Sat, Dec 12, 2009 at 02:09, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>
> Joel> I don't think it seems weird. I find this pretty nice, and since
> Joel> we already have printf, I wonder how hard it would be to implement
> Joel> this?
>
> Easy :-)
>
> Tom
>

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

---
 printcmd.c |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

--- a/printcmd.c
+++ b/printcmd.c
@@ -50,6 +50,8 @@
 #include "parser-defs.h"
 #include "charset.h"
 
+#include <ctype.h>
+
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et.al.   */
 #endif
@@ -2625,6 +2627,88 @@ printf_command (char *arg, int from_tty)
   do_cleanups (old_cleanups);
 }
 
+static void
+eval_command (char *exp, int from_tty)
+{
+#define CMDSIZE 1024
+  char *cmd;
+  char *cmdp;
+  int is_eval = 0;
+  char *eval_begin;
+
+  if (!exp)
+    return;
+
+  cmd = xmalloc (CMDSIZE + 1);
+  make_cleanup (xfree, cmd);
+  cmdp = cmd;
+
+  while (cmdp - cmd < CMDSIZE)
+    {
+      if (is_eval)
+        {
+          if (!exp[0] || exp[0]== '"')
+            {
+              struct value *value;
+              gdb_byte *buffer;
+              int length = -1;
+              struct type *char_type = NULL;
+              const char *la_encoding = NULL;
+              char tmp = exp[0];
+
+              exp[0] = '\0';
+              value = parse_and_eval (eval_begin);
+
+              switch (TYPE_CODE (value_type (value)))
+                {
+                  case TYPE_CODE_ARRAY:
+                    LA_GET_STRING (value, &buffer, &length,
+                                   &char_type, &la_encoding);
+                   break;
+                  case TYPE_CODE_INT:
+                    buffer = plongest (value_as_long (value));
+                    length = strlen (buffer);
+                    break;
+                  default:
+                    buffer = eval_begin;
+                    length = exp - eval_begin;
+                    break;
+                }
+
+              if (length > CMDSIZE - (cmdp - cmd))
+                length = CMDSIZE - (cmdp - cmd);
+              memcpy (cmdp, buffer, length);
+              cmdp += length;
+
+              exp[0] = tmp;
+              is_eval = 0;
+            }
+
+          exp ++;
+        }
+      else
+        {
+          if (!exp[0])
+            break;
+
+          if (exp[0] == '"')
+            {
+              is_eval = 1;
+              eval_begin = exp + 1;
+            }
+          else
+            {
+              cmdp[0] = exp[0];
+              cmdp ++;
+            }
+          exp ++;
+        }
+    }
+  cmdp[0] = '\0';
+
+  execute_command (cmd, from_tty);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2788,4 +2872,7 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Call command with variable."));
 }

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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-12  8:33                         ` Hui Zhu
@ 2009-12-13  4:05                           ` Hui Zhu
  2009-12-14 17:09                           ` Tom Tromey
  1 sibling, 0 replies; 39+ messages in thread
From: Hui Zhu @ 2009-12-13  4:05 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, Michael Snyder, gdb-patches ml

It just to show my idea, if you like it.  I will post new patch for it.

Thanks,
Hui

On Sat, Dec 12, 2009 at 16:33, Hui Zhu <teawater@gmail.com> wrote:
> Hi guys,
>
> I change it to:
> eval echo "++$a"
>
> Thanks,
> Hui
>
> On Sat, Dec 12, 2009 at 02:09, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>>
>> Joel> I don't think it seems weird. I find this pretty nice, and since
>> Joel> we already have printf, I wonder how hard it would be to implement
>> Joel> this?
>>
>> Easy :-)
>>
>> Tom
>>
>


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-12  8:33                         ` Hui Zhu
  2009-12-13  4:05                           ` Hui Zhu
@ 2009-12-14 17:09                           ` Tom Tromey
  2009-12-15  1:57                             ` Hui Zhu
  1 sibling, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2009-12-14 17:09 UTC (permalink / raw)
  To: Hui Zhu; +Cc: Joel Brobecker, Michael Snyder, gdb-patches ml

>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:

>> I change it to:
>> eval echo "++$a"

I'm afraid I don't like this approach either.

I think quoting expressions is very ugly.  Nothing else in gdb does
this.  Also, AFAIK it breaks completion.  Furthermore, this
implementation ignores the possibility of quotes embedded in an
expression.

Please choose some other approach.

thanks,
Tom


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-14 17:09                           ` Tom Tromey
@ 2009-12-15  1:57                             ` Hui Zhu
  2009-12-15 19:21                               ` Tom Tromey
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2009-12-15  1:57 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, Michael Snyder, gdb-patches ml

Agree with you.  Add " with variable make it oddness.

What about change it to others, like:
eval echo {++$a}

Or add " to simple string, like:
eval echo ++$a".core"
Or
eval echo ++$a'.core'

Thanks,
Hui



On Tue, Dec 15, 2009 at 01:09, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>
>>> I change it to:
>>> eval echo "++$a"
>
> I'm afraid I don't like this approach either.
>
> I think quoting expressions is very ugly.  Nothing else in gdb does
> this.  Also, AFAIK it breaks completion.  Furthermore, this
> implementation ignores the possibility of quotes embedded in an
> expression.
>
> Please choose some other approach.
>
> thanks,
> Tom
>


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-15  1:57                             ` Hui Zhu
@ 2009-12-15 19:21                               ` Tom Tromey
  2009-12-16  3:58                                 ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2009-12-15 19:21 UTC (permalink / raw)
  To: Hui Zhu; +Cc: Joel Brobecker, Michael Snyder, gdb-patches ml

>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:

>> Agree with you.  Add " with variable make it oddness.
>> What about change it to others, like:
>> eval echo {++$a}

>> Or add " to simple string, like:
>> eval echo ++$a".core"
>> Or
>> eval echo ++$a'.core'

What do you think of the idea of making it printf-like?

Some other approach, like the above two, would be ok with me, provided
that quoting is treated properly -- it should be possible to write any
command using eval.  So, there must be some way to quote whatever
special characters are used by eval.

Tom


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-15 19:21                               ` Tom Tromey
@ 2009-12-16  3:58                                 ` Hui Zhu
  2009-12-16 15:49                                   ` Stan Shebs
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2009-12-16  3:58 UTC (permalink / raw)
  To: tromey; +Cc: Joel Brobecker, Michael Snyder, gdb-patches ml

On Wed, Dec 16, 2009 at 03:20, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>
>>> Agree with you.  Add " with variable make it oddness.
>>> What about change it to others, like:
>>> eval echo {++$a}
>
>>> Or add " to simple string, like:
>>> eval echo ++$a".core"
>>> Or
>>> eval echo ++$a'.core'
>
> What do you think of the idea of making it printf-like?

It's not bad.  But need a lot of extend work that the old patch don't have.
And this command doesn't need convert a value from 1 type to another.
So I want use the "" way.


>
> Some other approach, like the above two, would be ok with me, provided
> that quoting is treated properly -- it should be possible to write any
> command using eval.  So, there must be some way to quote whatever
> special characters are used by eval.
>

If use "" the point the normail string, just the " and \ are the
special characters.
If we want add " in normal string.  Use \"
If we want add / in normal string.  Use \\
Do you think it's OK?

Thanks,
Hui


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-16  3:58                                 ` Hui Zhu
@ 2009-12-16 15:49                                   ` Stan Shebs
  2009-12-17  2:45                                     ` Hui Zhu
  2009-12-21 20:15                                     ` Tom Tromey
  0 siblings, 2 replies; 39+ messages in thread
From: Stan Shebs @ 2009-12-16 15:49 UTC (permalink / raw)
  To: Hui Zhu; +Cc: tromey, Joel Brobecker, Michael Snyder, gdb-patches ml

Hui Zhu wrote:
> On Wed, Dec 16, 2009 at 03:20, Tom Tromey <tromey@redhat.com> wrote:
>   
>>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>>>>>>>               
>>>> Agree with you.  Add " with variable make it oddness.
>>>> What about change it to others, like:
>>>> eval echo {++$a}
>>>>         
>>>> Or add " to simple string, like:
>>>> eval echo ++$a".core"
>>>> Or
>>>> eval echo ++$a'.core'
>>>>         
>> What do you think of the idea of making it printf-like?
>>     
>
> It's not bad.  But need a lot of extend work that the old patch don't have.
> And this command doesn't need convert a value from 1 type to another.
> So I want use the "" way.
>   
BTW, Pedro nudges me out of my stupor and reminds me that the 
soon-to-be-posted tracepoint action to evaluate without collecting is 
also called "eval" (it was originally proposed as "do" but that 
ambiguates with "down", which seemed like a bad idea).

The two versions are not necessarily mutually exclusive - the 
downloading at the start of a trace run gives us a chance to filter out 
eval's that don't make sense for the target agent - but if we go too 
afield on syntax (the tracepoint version is simply a comma-separated 
list of GDB expressions), then that's going to be more of a problem to 
reconcile.

Stan


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-16 15:49                                   ` Stan Shebs
@ 2009-12-17  2:45                                     ` Hui Zhu
  2009-12-17  4:26                                       ` Joel Brobecker
  2009-12-21 20:15                                     ` Tom Tromey
  1 sibling, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2009-12-17  2:45 UTC (permalink / raw)
  To: Stan Shebs; +Cc: tromey, Joel Brobecker, Michael Snyder, gdb-patches ml

OK.  We can change "eval" to other cmd.   What about "ceval"?

Hui

On Wed, Dec 16, 2009 at 23:49, Stan Shebs <stan@codesourcery.com> wrote:
> Hui Zhu wrote:
>>
>> On Wed, Dec 16, 2009 at 03:20, Tom Tromey <tromey@redhat.com> wrote:
>>
>>>>>>>>
>>>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>>>>>>>>
>>>>>
>>>>> Agree with you.  Add " with variable make it oddness.
>>>>> What about change it to others, like:
>>>>> eval echo {++$a}
>>>>>        Or add " to simple string, like:
>>>>> eval echo ++$a".core"
>>>>> Or
>>>>> eval echo ++$a'.core'
>>>>>
>>>
>>> What do you think of the idea of making it printf-like?
>>>
>>
>> It's not bad.  But need a lot of extend work that the old patch don't
>> have.
>> And this command doesn't need convert a value from 1 type to another.
>> So I want use the "" way.
>>
>
> BTW, Pedro nudges me out of my stupor and reminds me that the
> soon-to-be-posted tracepoint action to evaluate without collecting is also
> called "eval" (it was originally proposed as "do" but that ambiguates with
> "down", which seemed like a bad idea).
>
> The two versions are not necessarily mutually exclusive - the downloading at
> the start of a trace run gives us a chance to filter out eval's that don't
> make sense for the target agent - but if we go too afield on syntax (the
> tracepoint version is simply a comma-separated list of GDB expressions),
> then that's going to be more of a problem to reconcile.
>
> Stan
>
>


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-17  2:45                                     ` Hui Zhu
@ 2009-12-17  4:26                                       ` Joel Brobecker
  2009-12-17  4:29                                         ` Michael Snyder
  0 siblings, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2009-12-17  4:26 UTC (permalink / raw)
  To: Hui Zhu; +Cc: Stan Shebs, tromey, Michael Snyder, gdb-patches ml

> OK.  We can change "eval" to other cmd.   What about "ceval"?

eval is really a natural command name. Can we perhaps think of
another command name for the tracepoint stuff?

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-17  4:26                                       ` Joel Brobecker
@ 2009-12-17  4:29                                         ` Michael Snyder
  2009-12-17  9:32                                           ` Andreas Schwab
  0 siblings, 1 reply; 39+ messages in thread
From: Michael Snyder @ 2009-12-17  4:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Hui Zhu, Stan Shebs, tromey, gdb-patches ml

Joel Brobecker wrote:
>> OK.  We can change "eval" to other cmd.   What about "ceval"?
> 
> eval is really a natural command name. Can we perhaps think of
> another command name for the tracepoint stuff?
> 

Like "collect", "eval" in the tracepoint context is a command
that does not make any sense in any other context.

Maybe we can just decide from context which command we mean.


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-17  4:29                                         ` Michael Snyder
@ 2009-12-17  9:32                                           ` Andreas Schwab
  0 siblings, 0 replies; 39+ messages in thread
From: Andreas Schwab @ 2009-12-17  9:32 UTC (permalink / raw)
  To: Michael Snyder
  Cc: Joel Brobecker, Hui Zhu, Stan Shebs, tromey, gdb-patches ml

Michael Snyder <msnyder@vmware.com> writes:

> Joel Brobecker wrote:
>>> OK.  We can change "eval" to other cmd.   What about "ceval"?
>>
>> eval is really a natural command name. Can we perhaps think of
>> another command name for the tracepoint stuff?
>>
>
> Like "collect", "eval" in the tracepoint context is a command
> that does not make any sense in any other context.
>
> Maybe we can just decide from context which command we mean.

GDB supports the concept of sub commands, which is where the context
could come from.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-16 15:49                                   ` Stan Shebs
  2009-12-17  2:45                                     ` Hui Zhu
@ 2009-12-21 20:15                                     ` Tom Tromey
  2009-12-31  0:18                                       ` Stan Shebs
  1 sibling, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2009-12-21 20:15 UTC (permalink / raw)
  To: Stan Shebs; +Cc: Hui Zhu, Joel Brobecker, Michael Snyder, gdb-patches ml

>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:

Stan> BTW, Pedro nudges me out of my stupor and reminds me that the
Stan> soon-to-be-posted tracepoint action to evaluate without collecting is
Stan> also called "eval" (it was originally proposed as "do" but that
Stan> ambiguates with "down", which seemed like a bad idea).

Stan> The two versions are not necessarily mutually exclusive - the
Stan> downloading at the start of a trace run gives us a chance to filter
Stan> out eval's that don't make sense for the target agent - but if we go
Stan> too afield on syntax (the tracepoint version is simply a
Stan> comma-separated list of GDB expressions), then that's going to be more
Stan> of a problem to reconcile.

"eval" seems awfully generic for a command which is specific to
tracepoints.

I'm not super familiar with tracepoints but a lot of the other commands
seem to start with "t".  Why not "teval"?

Tom


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-21 20:15                                     ` Tom Tromey
@ 2009-12-31  0:18                                       ` Stan Shebs
  2010-01-04 14:42                                         ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Stan Shebs @ 2009-12-31  0:18 UTC (permalink / raw)
  To: Tom Tromey
  Cc: Stan Shebs, Hui Zhu, Joel Brobecker, Michael Snyder, gdb-patches ml

Tom Tromey wrote:
>>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:
>>>>>>             
>
> Stan> BTW, Pedro nudges me out of my stupor and reminds me that the
> Stan> soon-to-be-posted tracepoint action to evaluate without collecting is
> Stan> also called "eval" (it was originally proposed as "do" but that
> Stan> ambiguates with "down", which seemed like a bad idea).
>
> Stan> The two versions are not necessarily mutually exclusive - the
> Stan> downloading at the start of a trace run gives us a chance to filter
> Stan> out eval's that don't make sense for the target agent - but if we go
> Stan> too afield on syntax (the tracepoint version is simply a
> Stan> comma-separated list of GDB expressions), then that's going to be more
> Stan> of a problem to reconcile.
>
> "eval" seems awfully generic for a command which is specific to
> tracepoints.
>   
> I'm not super familiar with tracepoints but a lot of the other commands
> seem to start with "t".  Why not "teval"?
>   
That's a good idea.  If we ever come up with a Grand Unified Semantics 
of actions and commands for which generic "eval" matches tracepoint 
"teval", we can simply alias the two.

Stan


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2009-12-31  0:18                                       ` Stan Shebs
@ 2010-01-04 14:42                                         ` Hui Zhu
  2010-01-06  6:57                                           ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2010-01-04 14:42 UTC (permalink / raw)
  To: Stan Shebs
  Cc: Tom Tromey, Stan Shebs, Joel Brobecker, Michael Snyder, gdb-patches ml

Sorry for my poor English, did you mean that we can use "eval" for this command?

Thanks,
Hui

On Thu, Dec 31, 2009 at 08:18, Stan Shebs <stanshebs@earthlink.net> wrote:
> Tom Tromey wrote:
>>>>>>>
>>>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:
>>>>>>>
>>
>> Stan> BTW, Pedro nudges me out of my stupor and reminds me that the
>> Stan> soon-to-be-posted tracepoint action to evaluate without collecting
>> is
>> Stan> also called "eval" (it was originally proposed as "do" but that
>> Stan> ambiguates with "down", which seemed like a bad idea).
>>
>> Stan> The two versions are not necessarily mutually exclusive - the
>> Stan> downloading at the start of a trace run gives us a chance to filter
>> Stan> out eval's that don't make sense for the target agent - but if we go
>> Stan> too afield on syntax (the tracepoint version is simply a
>> Stan> comma-separated list of GDB expressions), then that's going to be
>> more
>> Stan> of a problem to reconcile.
>>
>> "eval" seems awfully generic for a command which is specific to
>> tracepoints.
>>  I'm not super familiar with tracepoints but a lot of the other commands
>> seem to start with "t".  Why not "teval"?
>>
>
> That's a good idea.  If we ever come up with a Grand Unified Semantics of
> actions and commands for which generic "eval" matches tracepoint "teval", we
> can simply alias the two.
>
> Stan
>
>


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-04 14:42                                         ` Hui Zhu
@ 2010-01-06  6:57                                           ` Hui Zhu
  2010-01-06  7:58                                             ` Joel Brobecker
  2010-01-06 10:13                                             ` Eli Zaretskii
  0 siblings, 2 replies; 39+ messages in thread
From: Hui Zhu @ 2010-01-06  6:57 UTC (permalink / raw)
  To: Stan Shebs, Tom Tromey, Stan Shebs, Joel Brobecker, Michael Snyder
  Cc: gdb-patches ml

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

Hello,

I make a new patch that eval command use "" to point out the simple command.
If want add " in normal string.  Use \"
If we want add \ in normal string.  Use \\
It can be use like:
(gdb) eval "echo \""$a++"\""
"7"(gdb) eval "echo \""$a++"\""
"8"(gdb) eval "echo \""$a++"\""
"9"(gdb) eval "echo \""$a++"\""
"10"(gdb) eval "echo \""$a++"\""
"11"(gdb) eval "echo \""$a++"\""
"12"(gdb) eval "echo \""$a++"\""
"13"(gdb) eval "echo \""$a++"\""

Please help me review it.

Best regards,
Hui

2010-01-06  Hui Zhu  <teawater@gmail.com>
	* printcmd.c (ctype.h): New include.
	(eval_command): New function.
	(_initialize_printcmd): New command "eval".



On Mon, Jan 4, 2010 at 22:42, Hui Zhu <teawater@gmail.com> wrote:
> Sorry for my poor English, did you mean that we can use "eval" for this command?
>
> Thanks,
> Hui
>
> On Thu, Dec 31, 2009 at 08:18, Stan Shebs <stanshebs@earthlink.net> wrote:
>> Tom Tromey wrote:
>>>>>>>>
>>>>>>>> "Stan" == Stan Shebs <stan@codesourcery.com> writes:
>>>>>>>>
>>>
>>> Stan> BTW, Pedro nudges me out of my stupor and reminds me that the
>>> Stan> soon-to-be-posted tracepoint action to evaluate without collecting
>>> is
>>> Stan> also called "eval" (it was originally proposed as "do" but that
>>> Stan> ambiguates with "down", which seemed like a bad idea).
>>>
>>> Stan> The two versions are not necessarily mutually exclusive - the
>>> Stan> downloading at the start of a trace run gives us a chance to filter
>>> Stan> out eval's that don't make sense for the target agent - but if we go
>>> Stan> too afield on syntax (the tracepoint version is simply a
>>> Stan> comma-separated list of GDB expressions), then that's going to be
>>> more
>>> Stan> of a problem to reconcile.
>>>
>>> "eval" seems awfully generic for a command which is specific to
>>> tracepoints.
>>>  I'm not super familiar with tracepoints but a lot of the other commands
>>> seem to start with "t".  Why not "teval"?
>>>
>>
>> That's a good idea.  If we ever come up with a Grand Unified Semantics of
>> actions and commands for which generic "eval" matches tracepoint "teval", we
>> can simply alias the two.
>>
>> Stan
>>
>>
>

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

---
 printcmd.c |  103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

--- a/printcmd.c
+++ b/printcmd.c
@@ -51,6 +51,8 @@
 #include "charset.h"
 #include "arch-utils.h"
 
+#include <ctype.h>
+
 #ifdef TUI
 #include "tui/tui.h"		/* For tui_active et.al.   */
 #endif
@@ -2649,6 +2651,104 @@ printf_command (char *arg, int from_tty)
   do_cleanups (old_cleanups);
 }
 
+static void
+eval_command (char *exp, int from_tty)
+{
+#define CMDSIZE 1024
+  char cmd[CMDSIZE + 1];
+  char *cmdp = cmd;
+  int is_eval = 1;
+  int is_backslash = 0;
+  int is_quote = 0;
+  char *eval_begin = exp;
+
+  if (!exp)
+    return;
+
+  while (cmdp - cmd < CMDSIZE)
+    {
+      if (!is_backslash && exp[0] == '\\')
+        {
+          is_backslash = 1;
+          exp ++;
+          continue;
+        }
+
+      if (is_eval)
+        {
+          if (!exp[0] || (exp[0]== '"' && !is_backslash))
+            {
+              struct value *value;
+              gdb_byte *buffer;
+              int length = -1;
+              struct type *char_type = NULL;
+              const char *la_encoding = NULL;
+              int is_end = 0;
+
+              if (!exp[0])
+                is_end = 1;
+              else
+                exp[0] = '\0';
+              if (strlen (eval_begin))
+                {
+                  value = parse_and_eval (eval_begin);
+
+                  switch (TYPE_CODE (value_type (value)))
+                    {
+                      case TYPE_CODE_ARRAY:
+                        LA_GET_STRING (value, &buffer, &length,
+                                       &char_type, &la_encoding);
+                       break;
+                      case TYPE_CODE_INT:
+                        buffer = plongest (value_as_long (value));
+                        length = strlen (buffer);
+                        break;
+                      default:
+                        buffer = eval_begin;
+                        length = exp - eval_begin;
+                        break;
+                    }
+
+                  if (length > CMDSIZE - (cmdp - cmd))
+                    length = CMDSIZE - (cmdp - cmd);
+                  memcpy (cmdp, buffer, length);
+                  cmdp += length;
+                }
+
+              if (is_end)
+                break;
+
+              is_eval = 0;
+            }
+
+          exp ++;
+        }
+      else
+        {
+          if (!exp[0])
+            break;
+
+          if (exp[0]== '"' && !is_backslash)
+            {
+              is_eval = 1;
+              eval_begin = exp + 1;
+            }
+          else
+            {
+              cmdp[0] = exp[0];
+              cmdp ++;
+            }
+          exp ++;
+        }
+
+      if (is_backslash)
+        is_backslash = 0;
+    }
+  cmdp[0] = '\0';
+
+  execute_command (cmd, from_tty);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2812,4 +2912,7 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Call command with variable."));
 }

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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-06  6:57                                           ` Hui Zhu
@ 2010-01-06  7:58                                             ` Joel Brobecker
  2010-01-09  9:55                                               ` Hui Zhu
  2010-01-06 10:13                                             ` Eli Zaretskii
  1 sibling, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2010-01-06  7:58 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder, gdb-patches ml

> 2010-01-06  Hui Zhu  <teawater@gmail.com>
> 	* printcmd.c (ctype.h): New include.
> 	(eval_command): New function.
> 	(_initialize_printcmd): New command "eval".

This is looking interesting :). A few comments:

  * what is the exact semantics of the eval command? I looked at
    the implementation, and I'm not I understand, or what I understand
    does not necessarily makes sense to me.

    So, in plain English first, what is the eval command expected
    to do, in particular, what parts and how will the command translate
    in the argument that gets passed.

  * implementation-wise:

> +#define CMDSIZE 1024
> +  char cmd[CMDSIZE + 1];

The GNU Coding Standard explicitly recommend against hard-coded
arbitrary limitations like these. In particular, your implementation
seems to just silently truncate the result if the user uses
an argument whose result does not fit in your CMDSIZE.

I think the implementation should allow for the argument to grow
to any size.

> +              if (strlen (eval_begin))
> +                {
> +                  value = parse_and_eval (eval_begin);
> +
> +                  switch (TYPE_CODE (value_type (value)))
> +                    {
> +                      case TYPE_CODE_ARRAY:
> +                        LA_GET_STRING (value, &buffer, &length,
> +                                       &char_type, &la_encoding);
> +                       break;
> +                      case TYPE_CODE_INT:
> +                        buffer = plongest (value_as_long (value));
> +                        length = strlen (buffer);
> +                        break;
> +                      default:
> +                        buffer = eval_begin;
> +                        length = exp - eval_begin;
> +                        break;
> +                    }
> +
> +                  if (length > CMDSIZE - (cmdp - cmd))
> +                    length = CMDSIZE - (cmdp - cmd);
> +                  memcpy (cmdp, buffer, length);
> +                  cmdp += length;

I think you should print "value_print" instead of doing the printing
yourself. Your implementation is missing a lot of other cases that
you need to handle (eg: TYPE_CODE_RANGE, just to mention one).


-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-06  6:57                                           ` Hui Zhu
  2010-01-06  7:58                                             ` Joel Brobecker
@ 2010-01-06 10:13                                             ` Eli Zaretskii
  1 sibling, 0 replies; 39+ messages in thread
From: Eli Zaretskii @ 2010-01-06 10:13 UTC (permalink / raw)
  To: Hui Zhu; +Cc: stanshebs, tromey, stan, brobecker, msnyder, gdb-patches

> From: Hui Zhu <teawater@gmail.com>
> Date: Wed, 6 Jan 2010 14:57:32 +0800
> Cc: gdb-patches ml <gdb-patches@sourceware.org>
> 
> I make a new patch that eval command use "" to point out the simple command.
> If want add " in normal string.  Use \"
> If we want add \ in normal string.  Use \\
> It can be use like:
> (gdb) eval "echo \""$a++"\""
> "7"(gdb) eval "echo \""$a++"\""
> "8"(gdb) eval "echo \""$a++"\""
> "9"(gdb) eval "echo \""$a++"\""
> "10"(gdb) eval "echo \""$a++"\""
> "11"(gdb) eval "echo \""$a++"\""
> "12"(gdb) eval "echo \""$a++"\""
> "13"(gdb) eval "echo \""$a++"\""
> 
> Please help me review it.

Thanks.  This doc string:

> +  add_com ("eval", no_class, eval_command, _("\
> +Call command with variable."));

Should be much more detailed, and in particular say when quotes should
be used and how to specify a literal quote character.

It also needs an update for the manual.

TIA


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-06  7:58                                             ` Joel Brobecker
@ 2010-01-09  9:55                                               ` Hui Zhu
  2010-01-09 10:56                                                 ` Joel Brobecker
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2010-01-09  9:55 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

Thanks Joel,

On Wed, Jan 6, 2010 at 15:57, Joel Brobecker <brobecker@adacore.com> wrote:
>> 2010-01-06  Hui Zhu  <teawater@gmail.com>
>>       * printcmd.c (ctype.h): New include.
>>       (eval_command): New function.
>>       (_initialize_printcmd): New command "eval".
>
> This is looking interesting :). A few comments:
>
>  * what is the exact semantics of the eval command? I looked at
>    the implementation, and I'm not I understand, or what I understand
>    does not necessarily makes sense to me.
>
>    So, in plain English first, what is the eval command expected
>    to do, in particular, what parts and how will the command translate
>    in the argument that gets passed.
>
>  * implementation-wise:
>
>> +#define CMDSIZE 1024
>> +  char cmd[CMDSIZE + 1];
>
> The GNU Coding Standard explicitly recommend against hard-coded
> arbitrary limitations like these. In particular, your implementation
> seems to just silently truncate the result if the user uses
> an argument whose result does not fit in your CMDSIZE.
>
> I think the implementation should allow for the argument to grow
> to any size.

It will be change.

>
>> +              if (strlen (eval_begin))
>> +                {
>> +                  value = parse_and_eval (eval_begin);
>> +
>> +                  switch (TYPE_CODE (value_type (value)))
>> +                    {
>> +                      case TYPE_CODE_ARRAY:
>> +                        LA_GET_STRING (value, &buffer, &length,
>> +                                       &char_type, &la_encoding);
>> +                       break;
>> +                      case TYPE_CODE_INT:
>> +                        buffer = plongest (value_as_long (value));
>> +                        length = strlen (buffer);
>> +                        break;
>> +                      default:
>> +                        buffer = eval_begin;
>> +                        length = exp - eval_begin;
>> +                        break;
>> +                    }
>> +
>> +                  if (length > CMDSIZE - (cmdp - cmd))
>> +                    length = CMDSIZE - (cmdp - cmd);
>> +                  memcpy (cmdp, buffer, length);
>> +                  cmdp += length;
>
> I think you should print "value_print" instead of doing the printing
> yourself. Your implementation is missing a lot of other cases that
> you need to handle (eg: TYPE_CODE_RANGE, just to mention one).
>

This value will be convert to string and  execute_command (cmd,
from_tty); will use it.
value_print print the value VAL in C-ish syntax on stream STREAM
according to OPTIONS.
But cannot find a example how to do it, could you help me with it?

Best regards,
Hui


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-09  9:55                                               ` Hui Zhu
@ 2010-01-09 10:56                                                 ` Joel Brobecker
  2010-01-09 15:18                                                   ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2010-01-09 10:56 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

> This value will be convert to string and  execute_command (cmd,
> from_tty); will use it.

Which parts of the argument will be expanded. To take another language
as an example, let's have a look at what a POSIX shell does:

        echo "1 $foo 2"

In double-quoted strings, the dollar sign has a the usual special
meaning introducing parameter expansion. As a result, if our
parameter "foo" was set to "bar",  the final value for the string above
would be "1 bar 2".

What I would like from you is a formal description of what your
command does. "This value will be convert to string" is too vague.
How will the conversion be done, and which parts of the string will
be evaluated, and which parts won't?

Intuitively, I think that you want to do the same as posix shells.
In other words, if I borrow and adjust part of the POSIX shell language
standard from the opengroup:

| Enclosing characters in double-quotes ( "" ) shall preserve the literal
| value of all characters within the double-quotes, with the exception of
| the characters dollar sign, and backslash, as follows:
| 
| $   The dollar sign shall retain its special meaning introducing
|     parameter expansion
| 
| \   The backslash shall retain its special meaning as an escape character
|     only when followed by one of the following characters:
| 
|         $   "   \   <newline>

The issue I'm having, which is leading to this discussion about
the semantics of your command, is that I do not think that the
implementation that you chose follows the semantics above.

> value_print print the value VAL in C-ish syntax on stream STREAM
> according to OPTIONS.
> But cannot find a example how to do it, could you help me with it?

In fact, it will print the value in the syntax of the current language.
I will fix the comment.

What you need to do, in order to convert your value into a string,
is create a memory ui_file, call value_print with your value and
the memory ui_file, and then extract the contents of your ui_file
as a string. See mem_fileopen and ui_file_xstrdup.

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-09 10:56                                                 ` Joel Brobecker
@ 2010-01-09 15:18                                                   ` Hui Zhu
  2010-01-10  5:43                                                     ` Joel Brobecker
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2010-01-09 15:18 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

Sorry Joel, I didn't write anything about the doc in prev mail because
I want write a special mail for it.

The current eval doesn't like shell.  It just use " point out the simple string.
And if the string is outside out ", eval will try to convert it to val.
The special chat is " and \
If want add " in string.  Use \"
If we want add \ in  string.  Use \\
For example:
eval "echo \""$a++"\""

"echo \"" and "\"" is simple string.
$a++ is val.

I know you want eval more like a language, but we already have python
support.  So  I suggest we use a simple eval.


On Sat, Jan 9, 2010 at 18:55, Joel Brobecker <brobecker@adacore.com> wrote:
>> This value will be convert to string and  execute_command (cmd,
>> from_tty); will use it.
>
> Which parts of the argument will be expanded. To take another language
> as an example, let's have a look at what a POSIX shell does:
>
>        echo "1 $foo 2"
>
> In double-quoted strings, the dollar sign has a the usual special
> meaning introducing parameter expansion. As a result, if our
> parameter "foo" was set to "bar",  the final value for the string above
> would be "1 bar 2".
>
> What I would like from you is a formal description of what your
> command does. "This value will be convert to string" is too vague.
> How will the conversion be done, and which parts of the string will
> be evaluated, and which parts won't?
>
> Intuitively, I think that you want to do the same as posix shells.
> In other words, if I borrow and adjust part of the POSIX shell language
> standard from the opengroup:
>
> | Enclosing characters in double-quotes ( "" ) shall preserve the literal
> | value of all characters within the double-quotes, with the exception of
> | the characters dollar sign, and backslash, as follows:
> |
> | $   The dollar sign shall retain its special meaning introducing
> |     parameter expansion
> |
> | \   The backslash shall retain its special meaning as an escape character
> |     only when followed by one of the following characters:
> |
> |         $   "   \   <newline>
>
> The issue I'm having, which is leading to this discussion about
> the semantics of your command, is that I do not think that the
> implementation that you chose follows the semantics above.
>
>> value_print print the value VAL in C-ish syntax on stream STREAM
>> according to OPTIONS.
>> But cannot find a example how to do it, could you help me with it?
>
> In fact, it will print the value in the syntax of the current language.
> I will fix the comment.
>
> What you need to do, in order to convert your value into a string,
> is create a memory ui_file, call value_print with your value and
> the memory ui_file, and then extract the contents of your ui_file
> as a string. See mem_fileopen and ui_file_xstrdup.
>

I got it.  Thanks.

Best regards,
Hui


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-09 15:18                                                   ` Hui Zhu
@ 2010-01-10  5:43                                                     ` Joel Brobecker
  2010-01-10 13:30                                                       ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2010-01-10  5:43 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

> The current eval doesn't like shell.  It just use " point out the
> simple string.  And if the string is outside out ", eval will try to
> convert it to val.

OK, I think I understand, now. I see pros and cons to your approach.
The approach I suggested (where we only expand convenience variables
by detecting the $ sign) is limited because it does not handle
expressions in general.  Your proposal, does, which is more powerful.

If the other maintainers like your syntax, then that'll be fine with me.

I have a counter proposal: Have eval follow the same syntax as the
printf command.  In other words:

    (gdb) eval "gcore filename.%s.%d", suffix, $counter++

That way, you save yourself having to implement yet-another-parser
in GDB, however simple. All you have to do, is factorize the
printf_command implementation into a function that stores the
string in a memory ui_file, and have both printf_command and
eval_command call that function.

And we save ourselves the trouble of have to explain the syntax
followed by the eval command, as it is identical to the syntax
used by the printf command.

How does that sound?

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-10  5:43                                                     ` Joel Brobecker
@ 2010-01-10 13:30                                                       ` Hui Zhu
  2010-01-10 14:00                                                         ` Joel Brobecker
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2010-01-10 13:30 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

Thanks Joel, I think the idea that you told is very good.

But I think we have know the type of the val that we want output, the
string we want it as string, the number we want it to be number.
Are you sure we need point out the type of val to be output?

And this idea is out of my ability.  Sorry about it.

Best regards,
Hui

On Sun, Jan 10, 2010 at 13:43, Joel Brobecker <brobecker@adacore.com> wrote:
>
> > The current eval doesn't like shell.  It just use " point out the
> > simple string.  And if the string is outside out ", eval will try to
> > convert it to val.
>
> OK, I think I understand, now. I see pros and cons to your approach.
> The approach I suggested (where we only expand convenience variables
> by detecting the $ sign) is limited because it does not handle
> expressions in general.  Your proposal, does, which is more powerful.
>
> If the other maintainers like your syntax, then that'll be fine with me.
>
> I have a counter proposal: Have eval follow the same syntax as the
> printf command.  In other words:
>
>    (gdb) eval "gcore filename.%s.%d", suffix, $counter++
>
> That way, you save yourself having to implement yet-another-parser
> in GDB, however simple. All you have to do, is factorize the
> printf_command implementation into a function that stores the
> string in a memory ui_file, and have both printf_command and
> eval_command call that function.
>
> And we save ourselves the trouble of have to explain the syntax
> followed by the eval command, as it is identical to the syntax
> used by the printf command.
>
> How does that sound?
>
> --
> Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-10 13:30                                                       ` Hui Zhu
@ 2010-01-10 14:00                                                         ` Joel Brobecker
  2010-01-10 14:16                                                           ` Hui Zhu
  2010-06-21 15:05                                                           ` Hui Zhu
  0 siblings, 2 replies; 39+ messages in thread
From: Joel Brobecker @ 2010-01-10 14:00 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

> But I think we have know the type of the val that we want output, the
> string we want it as string, the number we want it to be number.
> Are you sure we need point out the type of val to be output?
> 
> And this idea is out of my ability.  Sorry about it.

If you could have a look at how the "printf" command is implemented,
you might be surprised as to how easy this task might be.  For someone
who tackled the really complex project of grafting process record onto
GDB, I sincerely think that this should nearly be a no-brainer for you:

  1. Extract out the entire contents of the printf_command implementation
     into a separate function. Let's call it ui_printf:

       void
       ui_printf (char *args, ui_file *stream)

  2. Adjust the body so that all the printf/puts, etc, basically anything
     printing on stdout, are replace by equivalent calls that print in
     the given ui_file *stream;

  3. Implement the body of printf_command by simply calling this new
     function with gdb_stdout as the ui_file;

       printf_command (char *args, int from_tty)
       {
         ui_printf (args, gdb_stout);
       }

  4. Implement the eval function roughly like so:

        eval_command (char *args, int from_tty)
        {
          struct ui_file *ui_out = mem_fileopen ();
          char *expanded;
          struct cleanup *old_chain;

          ui_printf (args, ui_out);  // That's the new function extracted
                                     // from printf_command, which prints
                                     // in a ui_file instead of stdout.
          expanded = ui_file_xstrdup (ui_out, NULL);
          old_chain = make_cleanup (xfree, expanded);
          execute_command (expanded, from_tty);
          do_cleanups (old_chain);
        }

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-10 14:00                                                         ` Joel Brobecker
@ 2010-01-10 14:16                                                           ` Hui Zhu
  2010-06-21 15:05                                                           ` Hui Zhu
  1 sibling, 0 replies; 39+ messages in thread
From: Hui Zhu @ 2010-01-10 14:16 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

You are good on it.
Make a new patch is more easy than make me clear with this idea.  :)

Thanks,
Hui

On Sun, Jan 10, 2010 at 22:00, Joel Brobecker <brobecker@adacore.com> wrote:
>> But I think we have know the type of the val that we want output, the
>> string we want it as string, the number we want it to be number.
>> Are you sure we need point out the type of val to be output?
>>
>> And this idea is out of my ability.  Sorry about it.
>
> If you could have a look at how the "printf" command is implemented,
> you might be surprised as to how easy this task might be.  For someone
> who tackled the really complex project of grafting process record onto
> GDB, I sincerely think that this should nearly be a no-brainer for you:
>
>  1. Extract out the entire contents of the printf_command implementation
>     into a separate function. Let's call it ui_printf:
>
>       void
>       ui_printf (char *args, ui_file *stream)
>
>  2. Adjust the body so that all the printf/puts, etc, basically anything
>     printing on stdout, are replace by equivalent calls that print in
>     the given ui_file *stream;
>
>  3. Implement the body of printf_command by simply calling this new
>     function with gdb_stdout as the ui_file;
>
>       printf_command (char *args, int from_tty)
>       {
>         ui_printf (args, gdb_stout);
>       }
>
>  4. Implement the eval function roughly like so:
>
>        eval_command (char *args, int from_tty)
>        {
>          struct ui_file *ui_out = mem_fileopen ();
>          char *expanded;
>          struct cleanup *old_chain;
>
>          ui_printf (args, ui_out);  // That's the new function extracted
>                                     // from printf_command, which prints
>                                     // in a ui_file instead of stdout.
>          expanded = ui_file_xstrdup (ui_out, NULL);
>          old_chain = make_cleanup (xfree, expanded);
>          execute_command (expanded, from_tty);
>          do_cleanups (old_chain);
>        }
>
> --
> Joel
>


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-01-10 14:00                                                         ` Joel Brobecker
  2010-01-10 14:16                                                           ` Hui Zhu
@ 2010-06-21 15:05                                                           ` Hui Zhu
  2010-06-21 16:57                                                             ` Joel Brobecker
  2010-06-21 21:40                                                             ` Tom Tromey
  1 sibling, 2 replies; 39+ messages in thread
From: Hui Zhu @ 2010-06-21 15:05 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

On Sun, Jan 10, 2010 at 22:00, Joel Brobecker <brobecker@adacore.com> wrote:
>> But I think we have know the type of the val that we want output, the
>> string we want it as string, the number we want it to be number.
>> Are you sure we need point out the type of val to be output?
>>
>> And this idea is out of my ability.  Sorry about it.
>
> If you could have a look at how the "printf" command is implemented,
> you might be surprised as to how easy this task might be.  For someone
> who tackled the really complex project of grafting process record onto
> GDB, I sincerely think that this should nearly be a no-brainer for you:
>
>  1. Extract out the entire contents of the printf_command implementation
>     into a separate function. Let's call it ui_printf:
>
>       void
>       ui_printf (char *args, ui_file *stream)
>
>  2. Adjust the body so that all the printf/puts, etc, basically anything
>     printing on stdout, are replace by equivalent calls that print in
>     the given ui_file *stream;
>
>  3. Implement the body of printf_command by simply calling this new
>     function with gdb_stdout as the ui_file;
>
>       printf_command (char *args, int from_tty)
>       {
>         ui_printf (args, gdb_stout);
>       }
>
>  4. Implement the eval function roughly like so:
>
>        eval_command (char *args, int from_tty)
>        {
>          struct ui_file *ui_out = mem_fileopen ();
>          char *expanded;
>          struct cleanup *old_chain;
>
>          ui_printf (args, ui_out);  // That's the new function extracted
>                                     // from printf_command, which prints
>                                     // in a ui_file instead of stdout.
>          expanded = ui_file_xstrdup (ui_out, NULL);
>          old_chain = make_cleanup (xfree, expanded);
>          execute_command (expanded, from_tty);
>          do_cleanups (old_chain);
>        }
>
> --
> Joel
>

Hi Joel,

I make a new patch according to your mail.
Please help me review it.

Thanks,
Hui

2010-06-21  Hui Zhu  <teawater@gmail.com>

       * printcmd.c (ui_printf_maybe_filtered): New function.
       (printf_command): Call ui_printf_maybe_filtered.
       (_initialize_printcmd): New command "eval".


---
 printcmd.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 82 insertions(+), 14 deletions(-)

--- a/printcmd.c
+++ b/printcmd.c
@@ -1957,7 +1957,7 @@ print_variable_and_value (const char *na
 }

 static void
-printf_command (char *arg, int from_tty)
+ui_printf_maybe_filtered (char *arg, struct ui_file *stream, int filter)
 {
   char *f = NULL;
   char *s = arg;
@@ -2340,7 +2340,10 @@ printf_command (char *arg, int from_tty)
 		read_memory (tem, str, j);
 	      str[j] = 0;

-	      printf_filtered (current_substring, (char *) str);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, (char *) str);
+              else
+                fprintf_unfiltered (stream, current_substring, (char *) str);
 	    }
 	    break;
 	  case wide_string_arg:
@@ -2384,7 +2387,12 @@ printf_command (char *arg, int from_tty)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");

-	      printf_filtered (current_substring, obstack_base (&output));
+              if (filter)
+	        fprintf_filtered (stream, current_substring,
+                                  obstack_base (&output));
+              else
+                fprintf_unfiltered (stream, current_substring,
+                                    obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2416,7 +2424,12 @@ printf_command (char *arg, int from_tty)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");

-	      printf_filtered (current_substring, obstack_base (&output));
+              if (filter)
+	        fprintf_filtered (stream, current_substring,
+                                  obstack_base (&output));
+              else
+	        fprintf_unfiltered (stream, current_substring,
+                                    obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2433,7 +2446,10 @@ printf_command (char *arg, int from_tty)
 	      if (inv)
 		error (_("Invalid floating value found in program."));

-	      printf_filtered (current_substring, (double) val);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, (double) val);
+              else
+                fprintf_unfiltered (stream, current_substring, (double) val);
 	      break;
 	    }
 	  case long_double_arg:
@@ -2450,7 +2466,12 @@ printf_command (char *arg, int from_tty)
 	      if (inv)
 		error (_("Invalid floating value found in program."));

-	      printf_filtered (current_substring, (long double) val);
+              if (filter)
+	        fprintf_filtered (stream, current_substring,
+                                  (long double) val);
+              else
+	        fprintf_unfiltered (stream, current_substring,
+                                    (long double) val);
 	      break;
 	    }
 #else
@@ -2461,7 +2482,10 @@ printf_command (char *arg, int from_tty)
 	    {
 	      long long val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, val);
+              else
+                fprintf_unfiltered (stream, current_substring, val);
 	      break;
 	    }
 #else
@@ -2471,14 +2495,20 @@ printf_command (char *arg, int from_tty)
 	    {
 	      int val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, val);
+              else
+                fprintf_unfiltered (stream, current_substring, val);
 	      break;
 	    }
 	  case long_arg:
 	    {
 	      long val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, val);
+              else
+                fprintf_unfiltered (stream, current_substring, val);
 	      break;
 	    }

@@ -2490,7 +2520,10 @@ printf_command (char *arg, int from_tty)
 #if defined (PRINTF_HAS_DECFLOAT)
 	      /* If we have native support for Decimal floating
 		 printing, handle it here.  */
-	      printf_filtered (current_substring, param_ptr);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, param_ptr);
+              else
+                fprintf_unfiltered (stream, current_substring, param_ptr);
 #else

 	      /* As a workaround until vasprintf has native support for DFP
@@ -2579,7 +2612,10 @@ printf_command (char *arg, int from_tty)
 	      decimal_to_string (dfp_ptr, dfp_len, byte_order, decstr);

 	      /* Print the DFP value.  */
-	      printf_filtered (current_substring, decstr);
+              if (filter)
+	        fprintf_filtered (stream, current_substring, decstr);
+              else
+                fprintf_unfiltered (stream, current_substring, decstr);

 	      break;
 #endif
@@ -2634,13 +2670,19 @@ printf_command (char *arg, int from_tty)
 		  *fmt_p++ = 'l';
 		  *fmt_p++ = 'x';
 		  *fmt_p++ = '\0';
-		  printf_filtered (fmt, val);
+                  if (filter)
+		    fprintf_filtered (stream, fmt, val);
+                  else
+                    fprintf_unfiltered (stream, fmt, val);
 		}
 	      else
 		{
 		  *fmt_p++ = 's';
 		  *fmt_p++ = '\0';
-		  printf_filtered (fmt, "(nil)");
+                  if (filter)
+		    fprintf_filtered (stream, fmt, "(nil)");
+                  else
+                    fprintf_unfiltered (stream, fmt, "(nil)");
 		}

 	      break;
@@ -2658,11 +2700,34 @@ printf_command (char *arg, int from_tty)
        puts_filtered here.  Also, we pass a dummy argument because
        some platforms have modified GCC to include -Wformat-security
        by default, which will warn here if there is no argument.  */
-    printf_filtered (last_arg, 0);
+    if (filter)
+      fprintf_filtered (stream, last_arg, 0);
+    else
+      fprintf_unfiltered (stream, last_arg, 0);
   }
   do_cleanups (old_cleanups);
 }

+static void
+printf_command (char *arg, int from_tty)
+{
+  ui_printf_maybe_filtered (arg, gdb_stdout, 1);
+}
+
+static void
+eval_command (char *arg, int from_tty)
+{
+  struct ui_file *ui_out = mem_fileopen ();
+  char *expanded;
+  struct cleanup *old_chain;
+
+  ui_printf_maybe_filtered (arg, ui_out, 0);
+  expanded = ui_file_xstrdup (ui_out, NULL);
+  old_chain = make_cleanup (xfree, expanded);
+  execute_command (expanded, from_tty);
+  do_cleanups (old_chain);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2826,4 +2891,7 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Call command with variable."));
 }


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-21 15:05                                                           ` Hui Zhu
@ 2010-06-21 16:57                                                             ` Joel Brobecker
  2010-06-21 21:34                                                               ` Joel Brobecker
  2010-06-21 21:40                                                             ` Tom Tromey
  1 sibling, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2010-06-21 16:57 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

> 2010-06-21  Hui Zhu  <teawater@gmail.com>
> 
>        * printcmd.c (ui_printf_maybe_filtered): New function.
>        (printf_command): Call ui_printf_maybe_filtered.
>        (_initialize_printcmd): New command "eval".

This is close.

>  static void
> -printf_command (char *arg, int from_tty)
> +ui_printf_maybe_filtered (char *arg, struct ui_file *stream, int filter)

I would personally prefer it if we called this function "ui_printf",
dropping the "maybe_filtered". The "maybe_filtered" is just noise, IMO.

Also, this function needs a comment documenting its behavior.

> +static void
> +printf_command (char *arg, int from_tty)

Please add a comment as well. It can be as simple as ``Implement the
"print" command''.

> +static void
> +eval_command (char *arg, int from_tty)
> +{

And same here.

> +  add_com ("eval", no_class, eval_command, _("\
> +Call command with variable."));

This part will need to be approved by Eli. He's also ask for
documentation.

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-21 16:57                                                             ` Joel Brobecker
@ 2010-06-21 21:34                                                               ` Joel Brobecker
  0 siblings, 0 replies; 39+ messages in thread
From: Joel Brobecker @ 2010-06-21 21:34 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Stan Shebs, Tom Tromey, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

> > 2010-06-21  Hui Zhu  <teawater@gmail.com>
> > 
> >        * printcmd.c (ui_printf_maybe_filtered): New function.
> >        (printf_command): Call ui_printf_maybe_filtered.
> >        (_initialize_printcmd): New command "eval".

Something that I thought of afterwards: Can you also add a small test
for this new command. We should at least verify that a basic case
works fine - something like "eval echo [...]"...

Thanks,
-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-21 15:05                                                           ` Hui Zhu
  2010-06-21 16:57                                                             ` Joel Brobecker
@ 2010-06-21 21:40                                                             ` Tom Tromey
  2010-06-22  7:29                                                               ` Hui Zhu
  1 sibling, 1 reply; 39+ messages in thread
From: Tom Tromey @ 2010-06-21 21:40 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Joel Brobecker, Stan Shebs, Stan Shebs, Michael Snyder,
	gdb-patches ml, Eli Zaretskii

>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:

>>  static void
>> -printf_command (char *arg, int from_tty)
>> +ui_printf_maybe_filtered (char *arg, struct ui_file *stream, int filter)

I don't think you really need the 'filter' argument.

fprintf_filtered will actually work unfiltered unless printing to
gdb_stdout.  (One of several bad things about this API -- but we already
rely on it.)

I think this argument uglifies the code quite a bit.

>> +static void
>> +eval_command (char *arg, int from_tty)
>> +{
>> +  struct ui_file *ui_out = mem_fileopen ();
>> +  char *expanded;
>> +  struct cleanup *old_chain;
>> +
>> +  ui_printf_maybe_filtered (arg, ui_out, 0);
>> +  expanded = ui_file_xstrdup (ui_out, NULL);
>> +  old_chain = make_cleanup (xfree, expanded);
>> +  execute_command (expanded, from_tty);
>> +  do_cleanups (old_chain);

You need a cleanup in there to destroy the new ui_out.

>> +  add_com ("eval", no_class, eval_command, _("\
>> +Call command with variable."));
>>  }

This needs a better doc string.

I think this approach is good overall.  It needs a documentation patch,
a NEWS entry, and some tests.

Tom


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-21 21:40                                                             ` Tom Tromey
@ 2010-06-22  7:29                                                               ` Hui Zhu
  2010-06-22  9:50                                                                 ` Pedro Alves
                                                                                   ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: Hui Zhu @ 2010-06-22  7:29 UTC (permalink / raw)
  To: Tom Tromey, Joel Brobecker, Eli Zaretskii
  Cc: Stan Shebs, Stan Shebs, Michael Snyder, gdb-patches ml

On Tue, Jun 22, 2010 at 05:39, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>
>>>  static void
>>> -printf_command (char *arg, int from_tty)
>>> +ui_printf_maybe_filtered (char *arg, struct ui_file *stream, int filter)
>
> I don't think you really need the 'filter' argument.
>
> fprintf_filtered will actually work unfiltered unless printing to
> gdb_stdout.  (One of several bad things about this API -- but we already
> rely on it.)
>
> I think this argument uglifies the code quite a bit.
>
>>> +static void
>>> +eval_command (char *arg, int from_tty)
>>> +{
>>> +  struct ui_file *ui_out = mem_fileopen ();
>>> +  char *expanded;
>>> +  struct cleanup *old_chain;
>>> +
>>> +  ui_printf_maybe_filtered (arg, ui_out, 0);
>>> +  expanded = ui_file_xstrdup (ui_out, NULL);
>>> +  old_chain = make_cleanup (xfree, expanded);
>>> +  execute_command (expanded, from_tty);
>>> +  do_cleanups (old_chain);
>
> You need a cleanup in there to destroy the new ui_out.
>
>>> +  add_com ("eval", no_class, eval_command, _("\
>>> +Call command with variable."));
>>>  }
>
> This needs a better doc string.
>
> I think this approach is good overall.  It needs a documentation patch,
> a NEWS entry, and some tests.
>
> Tom
>

Hi guys,

I make a new patch according to your mails.

It include the code, doc and test.  Please help me review it.

Thanks,
Hui

2010-06-22  Hui Zhu  <teawater@gmail.com>

       * printcmd.c (ui_printf): New function.
       (printf_command): Call ui_printf.
       (_initialize_printcmd): New command "eval".

2010-06-22  Hui Zhu  <teawater@gmail.com>

	* gdb.texinfo: (Commands for Controlled Output): Add
	documentation for command "eval".

2010-06-22  Hui Zhu  <teawater@gmail.com>

	* gdb.base/eval.exp: New file.


---
 NEWS                        |    4 ++
 doc/gdb.texinfo             |    5 +++
 printcmd.c                  |   63 ++++++++++++++++++++++++++++++++++----------
 testsuite/gdb.base/eval.exp |   22 +++++++++++++++
 4 files changed, 80 insertions(+), 14 deletions(-)

--- a/NEWS
+++ b/NEWS
@@ -84,6 +84,10 @@ qRelocInsn

 * New commands

+eval template, expressions...
+  Convert the values of one or more expressions under the control
+  of the string template to a command line and call it.
+
 set target-file-system-kind unix|dos-based|auto
 show target-file-system-kind
   Set or show the assumed file system kind for target reported file
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -20073,6 +20073,11 @@ Here's an example of printing DFP types
 printf "D32: %Hf - D64: %Df - D128: %DDf\n",1.2345df,1.2E10dd,1.2E1dl
 @end smallexample

+@kindex eval
+@item eval @var{template}, @var{expressions}@dots{}
+Convert the values of one or more @var{expressions} under the control of
+the string @var{template} to a command line and call it.
+
 @end table

 @node Python
--- a/printcmd.c
+++ b/printcmd.c
@@ -1956,8 +1956,10 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }

+/* printf "printf format string" ARG to STREAM.  */
+
 static void
-printf_command (char *arg, int from_tty)
+ui_printf (char *arg, struct ui_file *stream)
 {
   char *f = NULL;
   char *s = arg;
@@ -2340,7 +2342,7 @@ printf_command (char *arg, int from_tty)
 		read_memory (tem, str, j);
 	      str[j] = 0;

-	      printf_filtered (current_substring, (char *) str);
+              fprintf_filtered (stream, current_substring, (char *) str);
 	    }
 	    break;
 	  case wide_string_arg:
@@ -2384,7 +2386,8 @@ printf_command (char *arg, int from_tty)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");

-	      printf_filtered (current_substring, obstack_base (&output));
+	      fprintf_filtered (stream, current_substring,
+                                obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2416,7 +2419,8 @@ printf_command (char *arg, int from_tty)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");

-	      printf_filtered (current_substring, obstack_base (&output));
+	      fprintf_filtered (stream, current_substring,
+                                obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2433,7 +2437,7 @@ printf_command (char *arg, int from_tty)
 	      if (inv)
 		error (_("Invalid floating value found in program."));

-	      printf_filtered (current_substring, (double) val);
+              fprintf_filtered (stream, current_substring, (double) val);
 	      break;
 	    }
 	  case long_double_arg:
@@ -2450,7 +2454,8 @@ printf_command (char *arg, int from_tty)
 	      if (inv)
 		error (_("Invalid floating value found in program."));

-	      printf_filtered (current_substring, (long double) val);
+	      fprintf_filtered (stream, current_substring,
+                                (long double) val);
 	      break;
 	    }
 #else
@@ -2461,7 +2466,7 @@ printf_command (char *arg, int from_tty)
 	    {
 	      long long val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              fprintf_filtered (stream, current_substring, val);
 	      break;
 	    }
 #else
@@ -2471,14 +2476,14 @@ printf_command (char *arg, int from_tty)
 	    {
 	      int val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              fprintf_filtered (stream, current_substring, val);
 	      break;
 	    }
 	  case long_arg:
 	    {
 	      long val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              fprintf_filtered (stream, current_substring, val);
 	      break;
 	    }

@@ -2490,7 +2495,7 @@ printf_command (char *arg, int from_tty)
 #if defined (PRINTF_HAS_DECFLOAT)
 	      /* If we have native support for Decimal floating
 		 printing, handle it here.  */
-	      printf_filtered (current_substring, param_ptr);
+              fprintf_filtered (stream, current_substring, param_ptr);
 #else

 	      /* As a workaround until vasprintf has native support for DFP
@@ -2579,7 +2584,7 @@ printf_command (char *arg, int from_tty)
 	      decimal_to_string (dfp_ptr, dfp_len, byte_order, decstr);

 	      /* Print the DFP value.  */
-	      printf_filtered (current_substring, decstr);
+              fprintf_filtered (stream, current_substring, decstr);

 	      break;
 #endif
@@ -2634,13 +2639,13 @@ printf_command (char *arg, int from_tty)
 		  *fmt_p++ = 'l';
 		  *fmt_p++ = 'x';
 		  *fmt_p++ = '\0';
-		  printf_filtered (fmt, val);
+                  fprintf_filtered (stream, fmt, val);
 		}
 	      else
 		{
 		  *fmt_p++ = 's';
 		  *fmt_p++ = '\0';
-		  printf_filtered (fmt, "(nil)");
+                  fprintf_filtered (stream, fmt, "(nil)");
 		}

 	      break;
@@ -2658,11 +2663,37 @@ printf_command (char *arg, int from_tty)
        puts_filtered here.  Also, we pass a dummy argument because
        some platforms have modified GCC to include -Wformat-security
        by default, which will warn here if there is no argument.  */
-    printf_filtered (last_arg, 0);
+    fprintf_filtered (stream, last_arg, 0);
   }
   do_cleanups (old_cleanups);
 }

+/* Implement the "printf" command.  */
+
+static void
+printf_command (char *arg, int from_tty)
+{
+  ui_printf (arg, gdb_stdout);
+}
+
+/* Implement the "eval" command.  */
+
+static void
+eval_command (char *arg, int from_tty)
+{
+  struct ui_file *ui_out = mem_fileopen ();
+  struct cleanup *ui_out_cleanups = make_cleanup_ui_file_delete (ui_out);
+  char *expanded;
+  struct cleanup *expanded_chain;
+
+  ui_printf (arg, ui_out);
+  expanded = ui_file_xstrdup (ui_out, NULL);
+  expanded_chain = make_cleanup (xfree, expanded);
+  execute_command (expanded, from_tty);
+  do_cleanups (expanded_chain);
+  do_cleanups (ui_out_cleanups);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2826,4 +2857,8 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Convert \"printf format string\", arg1, arg2, arg3, ..., argn to\n\
+a command line and call it."));
 }
--- /dev/null
+++ b/testsuite/gdb.base/eval.exp
@@ -0,0 +1,22 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+gdb_exit
+gdb_start
+
+gdb_test_no_output "set \$a = 10" "Initialize \$a."
+
+gdb_test "eval \"echo %d\\n\", \$a++" "10"
+gdb_test "eval \"echo %d\\n\", \$a++" "11"


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-22  7:29                                                               ` Hui Zhu
@ 2010-06-22  9:50                                                                 ` Pedro Alves
  2010-06-22 15:21                                                                 ` Joel Brobecker
  2010-06-22 18:05                                                                 ` Eli Zaretskii
  2 siblings, 0 replies; 39+ messages in thread
From: Pedro Alves @ 2010-06-22  9:50 UTC (permalink / raw)
  To: gdb-patches
  Cc: Hui Zhu, Tom Tromey, Joel Brobecker, Eli Zaretskii, Stan Shebs,
	Stan Shebs, Michael Snyder

On Tuesday 22 June 2010 08:29:03, Hui Zhu wrote:
> +gdb_test "eval \"echo %d\\n\", \$a++" "10"
> +gdb_test "eval \"echo %d\\n\", \$a++" "11"

As principle, please make sure each test has distinct output in gdb.sum.

-- 
Pedro Alves


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-22  7:29                                                               ` Hui Zhu
  2010-06-22  9:50                                                                 ` Pedro Alves
@ 2010-06-22 15:21                                                                 ` Joel Brobecker
  2010-06-22 18:05                                                                 ` Eli Zaretskii
  2 siblings, 0 replies; 39+ messages in thread
From: Joel Brobecker @ 2010-06-22 15:21 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Tom Tromey, Eli Zaretskii, Stan Shebs, Stan Shebs,
	Michael Snyder, gdb-patches ml

> +  do_cleanups (expanded_chain);
> +  do_cleanups (ui_out_cleanups);

The cleanup structure is actually like a stack. You just have to
store the first cleanup, and any cleanup after that will be performed.

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-22  7:29                                                               ` Hui Zhu
  2010-06-22  9:50                                                                 ` Pedro Alves
  2010-06-22 15:21                                                                 ` Joel Brobecker
@ 2010-06-22 18:05                                                                 ` Eli Zaretskii
  2010-06-23  2:03                                                                   ` Hui Zhu
  2 siblings, 1 reply; 39+ messages in thread
From: Eli Zaretskii @ 2010-06-22 18:05 UTC (permalink / raw)
  To: Hui Zhu; +Cc: tromey, brobecker, stanshebs, stan, msnyder, gdb-patches

> From: Hui Zhu <teawater@gmail.com>
> Date: Tue, 22 Jun 2010 15:29:03 +0800
> Cc: Stan Shebs <stanshebs@earthlink.net>, Stan Shebs <stan@codesourcery.com>, 
> 	Michael Snyder <msnyder@vmware.com>, gdb-patches ml <gdb-patches@sourceware.org>
> 
> I make a new patch according to your mails.
> 
> It include the code, doc and test.  Please help me review it.

Thanks.  The doc parts are okay, with these two comments:

> +eval template, expressions...
> +  Convert the values of one or more expressions under the control
> +  of the string template to a command line and call it.
                                             ^
A comma here, please.

> +@item eval @var{template}, @var{expressions}@dots{}
> +Convert the values of one or more @var{expressions} under the control of
> +the string @var{template} to a command line and call it.
                                              ^
Same here.


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-22 18:05                                                                 ` Eli Zaretskii
@ 2010-06-23  2:03                                                                   ` Hui Zhu
  2010-06-23 16:59                                                                     ` Joel Brobecker
  0 siblings, 1 reply; 39+ messages in thread
From: Hui Zhu @ 2010-06-23  2:03 UTC (permalink / raw)
  To: Eli Zaretskii, brobecker, Pedro Alves, Tom Tromey
  Cc: stanshebs, stan, msnyder, gdb-patches

On Wed, Jun 23, 2010 at 02:03, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Hui Zhu <teawater@gmail.com>
>> Date: Tue, 22 Jun 2010 15:29:03 +0800
>> Cc: Stan Shebs <stanshebs@earthlink.net>, Stan Shebs <stan@codesourcery.com>,
>>       Michael Snyder <msnyder@vmware.com>, gdb-patches ml <gdb-patches@sourceware.org>
>>
>> I make a new patch according to your mails.
>>
>> It include the code, doc and test.  Please help me review it.
>
> Thanks.  The doc parts are okay, with these two comments:
>
>> +eval template, expressions...
>> +  Convert the values of one or more expressions under the control
>> +  of the string template to a command line and call it.
>                                             ^
> A comma here, please.
>
>> +@item eval @var{template}, @var{expressions}@dots{}
>> +Convert the values of one or more @var{expressions} under the control of
>> +the string @var{template} to a command line and call it.
>                                              ^
> Same here.
>

Hi guys,

I make a new patch according to your mails.
Please help me review it.  Thanks.

Best,
Hui

2010-06-23  Hui Zhu  <teawater@gmail.com>

       * printcmd.c (ui_printf): New function.
       (printf_command): Call ui_printf.
       (_initialize_printcmd): New command "eval".

2010-06-23  Hui Zhu  <teawater@gmail.com>

	* gdb.texinfo: (Commands for Controlled Output): Add
	documentation for command "eval".

2010-06-23  Hui Zhu  <teawater@gmail.com>

	* gdb.base/eval.exp: New file.


---
 NEWS                        |    4 ++
 doc/gdb.texinfo             |    5 +++
 printcmd.c                  |   64 ++++++++++++++++++++++++++++++++++----------
 testsuite/gdb.base/eval.exp |   22 +++++++++++++++
 4 files changed, 81 insertions(+), 14 deletions(-)

--- a/NEWS
+++ b/NEWS
@@ -84,6 +84,10 @@ qRelocInsn

 * New commands

+eval template, expressions...
+  Convert the values of one or more expressions under the control
+  of the string template to a command line, and call it.
+
 set target-file-system-kind unix|dos-based|auto
 show target-file-system-kind
   Set or show the assumed file system kind for target reported file
--- a/doc/gdb.texinfo
+++ b/doc/gdb.texinfo
@@ -20073,6 +20073,11 @@ Here's an example of printing DFP types
 printf "D32: %Hf - D64: %Df - D128: %DDf\n",1.2345df,1.2E10dd,1.2E1dl
 @end smallexample

+@kindex eval
+@item eval @var{template}, @var{expressions}@dots{}
+Convert the values of one or more @var{expressions} under the control of
+the string @var{template} to a command line, and call it.
+
 @end table

 @node Python
--- a/printcmd.c
+++ b/printcmd.c
@@ -1956,8 +1956,10 @@ print_variable_and_value (const char *na
   fprintf_filtered (stream, "\n");
 }

+/* printf "printf format string" ARG to STREAM.  */
+
 static void
-printf_command (char *arg, int from_tty)
+ui_printf (char *arg, struct ui_file *stream)
 {
   char *f = NULL;
   char *s = arg;
@@ -2340,7 +2342,7 @@ printf_command (char *arg, int from_tty)
 		read_memory (tem, str, j);
 	      str[j] = 0;

-	      printf_filtered (current_substring, (char *) str);
+              fprintf_filtered (stream, current_substring, (char *) str);
 	    }
 	    break;
 	  case wide_string_arg:
@@ -2384,7 +2386,8 @@ printf_command (char *arg, int from_tty)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");

-	      printf_filtered (current_substring, obstack_base (&output));
+	      fprintf_filtered (stream, current_substring,
+                                obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2416,7 +2419,8 @@ printf_command (char *arg, int from_tty)
 					 &output, translit_char);
 	      obstack_grow_str0 (&output, "");

-	      printf_filtered (current_substring, obstack_base (&output));
+	      fprintf_filtered (stream, current_substring,
+                                obstack_base (&output));
 	      do_cleanups (inner_cleanup);
 	    }
 	    break;
@@ -2433,7 +2437,7 @@ printf_command (char *arg, int from_tty)
 	      if (inv)
 		error (_("Invalid floating value found in program."));

-	      printf_filtered (current_substring, (double) val);
+              fprintf_filtered (stream, current_substring, (double) val);
 	      break;
 	    }
 	  case long_double_arg:
@@ -2450,7 +2454,8 @@ printf_command (char *arg, int from_tty)
 	      if (inv)
 		error (_("Invalid floating value found in program."));

-	      printf_filtered (current_substring, (long double) val);
+	      fprintf_filtered (stream, current_substring,
+                                (long double) val);
 	      break;
 	    }
 #else
@@ -2461,7 +2466,7 @@ printf_command (char *arg, int from_tty)
 	    {
 	      long long val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              fprintf_filtered (stream, current_substring, val);
 	      break;
 	    }
 #else
@@ -2471,14 +2476,14 @@ printf_command (char *arg, int from_tty)
 	    {
 	      int val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              fprintf_filtered (stream, current_substring, val);
 	      break;
 	    }
 	  case long_arg:
 	    {
 	      long val = value_as_long (val_args[i]);

-	      printf_filtered (current_substring, val);
+              fprintf_filtered (stream, current_substring, val);
 	      break;
 	    }

@@ -2490,7 +2495,7 @@ printf_command (char *arg, int from_tty)
 #if defined (PRINTF_HAS_DECFLOAT)
 	      /* If we have native support for Decimal floating
 		 printing, handle it here.  */
-	      printf_filtered (current_substring, param_ptr);
+              fprintf_filtered (stream, current_substring, param_ptr);
 #else

 	      /* As a workaround until vasprintf has native support for DFP
@@ -2579,7 +2584,7 @@ printf_command (char *arg, int from_tty)
 	      decimal_to_string (dfp_ptr, dfp_len, byte_order, decstr);

 	      /* Print the DFP value.  */
-	      printf_filtered (current_substring, decstr);
+              fprintf_filtered (stream, current_substring, decstr);

 	      break;
 #endif
@@ -2634,13 +2639,13 @@ printf_command (char *arg, int from_tty)
 		  *fmt_p++ = 'l';
 		  *fmt_p++ = 'x';
 		  *fmt_p++ = '\0';
-		  printf_filtered (fmt, val);
+                  fprintf_filtered (stream, fmt, val);
 		}
 	      else
 		{
 		  *fmt_p++ = 's';
 		  *fmt_p++ = '\0';
-		  printf_filtered (fmt, "(nil)");
+                  fprintf_filtered (stream, fmt, "(nil)");
 		}

 	      break;
@@ -2658,11 +2663,38 @@ printf_command (char *arg, int from_tty)
        puts_filtered here.  Also, we pass a dummy argument because
        some platforms have modified GCC to include -Wformat-security
        by default, which will warn here if there is no argument.  */
-    printf_filtered (last_arg, 0);
+    fprintf_filtered (stream, last_arg, 0);
   }
   do_cleanups (old_cleanups);
 }

+/* Implement the "printf" command.  */
+
+static void
+printf_command (char *arg, int from_tty)
+{
+  ui_printf (arg, gdb_stdout);
+}
+
+/* Implement the "eval" command.  */
+
+static void
+eval_command (char *arg, int from_tty)
+{
+  struct ui_file *ui_out = mem_fileopen ();
+  struct cleanup *cleanups = make_cleanup_ui_file_delete (ui_out);
+  char *expanded;
+
+  ui_printf (arg, ui_out);
+
+  expanded = ui_file_xstrdup (ui_out, NULL);
+  make_cleanup (xfree, expanded);
+
+  execute_command (expanded, from_tty);
+
+  do_cleanups (cleanups);
+}
+
 void
 _initialize_printcmd (void)
 {
@@ -2826,4 +2858,8 @@ Show printing of source filename and lin
 			   NULL,
 			   show_print_symbol_filename,
 			   &setprintlist, &showprintlist);
+
+  add_com ("eval", no_class, eval_command, _("\
+Convert \"printf format string\", arg1, arg2, arg3, ..., argn to\n\
+a command line, and call it."));
 }
--- /dev/null
+++ b/testsuite/gdb.base/eval.exp
@@ -0,0 +1,22 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+gdb_exit
+gdb_start
+
+gdb_test_no_output "set \$a = 10" "Initialize \$a."
+
+gdb_test "eval \"echo %d\\n\", \$a++" "10"
+gdb_test "eval \"echo %d\\n\", ++\$a" "12"


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-23  2:03                                                                   ` Hui Zhu
@ 2010-06-23 16:59                                                                     ` Joel Brobecker
  2010-06-24  6:13                                                                       ` Hui Zhu
  0 siblings, 1 reply; 39+ messages in thread
From: Joel Brobecker @ 2010-06-23 16:59 UTC (permalink / raw)
  To: Hui Zhu
  Cc: Eli Zaretskii, Pedro Alves, Tom Tromey, stanshebs, stan, msnyder,
	gdb-patches

> 2010-06-23  Hui Zhu  <teawater@gmail.com>
> 
>        * printcmd.c (ui_printf): New function.
>        (printf_command): Call ui_printf.
>        (_initialize_printcmd): New command "eval".

This is OK.

> 2010-06-23  Hui Zhu  <teawater@gmail.com>
> 
> 	* gdb.base/eval.exp: New file.

This is also OK.  Just for the record, I first thought that your two
tests where still the same and thus would generate the same message.
But after a closer inspection, one used $a++ and the other ++$a.
Why not use something more different like '$a * 2' maybe?

Another way to make things different is to specify distinct messages
explicitly - for instance "first eval" / "second eval"...

-- 
Joel


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

* Re: [RFC] Let "gcore" command accept a suffix argument
  2010-06-23 16:59                                                                     ` Joel Brobecker
@ 2010-06-24  6:13                                                                       ` Hui Zhu
  0 siblings, 0 replies; 39+ messages in thread
From: Hui Zhu @ 2010-06-24  6:13 UTC (permalink / raw)
  To: Joel Brobecker
  Cc: Eli Zaretskii, Pedro Alves, Tom Tromey, stanshebs, stan, msnyder,
	gdb-patches

On Thu, Jun 24, 2010 at 00:59, Joel Brobecker <brobecker@adacore.com> wrote:
>> 2010-06-23  Hui Zhu  <teawater@gmail.com>
>>
>>        * printcmd.c (ui_printf): New function.
>>        (printf_command): Call ui_printf.
>>        (_initialize_printcmd): New command "eval".
>
> This is OK.
>
>> 2010-06-23  Hui Zhu  <teawater@gmail.com>
>>
>>       * gdb.base/eval.exp: New file.
>
> This is also OK.  Just for the record, I first thought that your two
> tests where still the same and thus would generate the same message.
> But after a closer inspection, one used $a++ and the other ++$a.
> Why not use something more different like '$a * 2' maybe?
>
> Another way to make things different is to specify distinct messages
> explicitly - for instance "first eval" / "second eval"...
>

Thanks Joel, Changed it to:
gdb_test "eval \"echo %d\\n\", \$a++" "10" "First eval."
gdb_test "eval \"echo %d\\n\", \$a*2" "22" "Second eval."

And checked in.

Best,
Hui


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

end of thread, other threads:[~2010-06-24  6:13 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <4B11DA3C.3000203@vmware.com>
     [not found] ` <daef60380911300437o4c616eb2v5ad7bfe99bd3c5e9@mail.gmail.com>
     [not found]   ` <20091130162246.GE4034@adacore.com>
     [not found]     ` <4B141157.3070709@vmware.com>
     [not found]       ` <20091130185341.GI4034@adacore.com>
     [not found]         ` <4B141469.5030402@vmware.com>
     [not found]           ` <20091130190619.GJ4034@adacore.com>
     [not found]             ` <4B1428F0.7090608@vmware.com>
     [not found]               ` <daef60380912091827u6ff7127bp1d03998a40932914@mail.gmail.com>
2009-12-10  2:32                 ` [RFC] Let "gcore" command accept a suffix argument Hui Zhu
2009-12-10 17:08                   ` Tom Tromey
2009-12-11 10:06                     ` Joel Brobecker
2009-12-11 18:25                       ` Tom Tromey
2009-12-12  8:33                         ` Hui Zhu
2009-12-13  4:05                           ` Hui Zhu
2009-12-14 17:09                           ` Tom Tromey
2009-12-15  1:57                             ` Hui Zhu
2009-12-15 19:21                               ` Tom Tromey
2009-12-16  3:58                                 ` Hui Zhu
2009-12-16 15:49                                   ` Stan Shebs
2009-12-17  2:45                                     ` Hui Zhu
2009-12-17  4:26                                       ` Joel Brobecker
2009-12-17  4:29                                         ` Michael Snyder
2009-12-17  9:32                                           ` Andreas Schwab
2009-12-21 20:15                                     ` Tom Tromey
2009-12-31  0:18                                       ` Stan Shebs
2010-01-04 14:42                                         ` Hui Zhu
2010-01-06  6:57                                           ` Hui Zhu
2010-01-06  7:58                                             ` Joel Brobecker
2010-01-09  9:55                                               ` Hui Zhu
2010-01-09 10:56                                                 ` Joel Brobecker
2010-01-09 15:18                                                   ` Hui Zhu
2010-01-10  5:43                                                     ` Joel Brobecker
2010-01-10 13:30                                                       ` Hui Zhu
2010-01-10 14:00                                                         ` Joel Brobecker
2010-01-10 14:16                                                           ` Hui Zhu
2010-06-21 15:05                                                           ` Hui Zhu
2010-06-21 16:57                                                             ` Joel Brobecker
2010-06-21 21:34                                                               ` Joel Brobecker
2010-06-21 21:40                                                             ` Tom Tromey
2010-06-22  7:29                                                               ` Hui Zhu
2010-06-22  9:50                                                                 ` Pedro Alves
2010-06-22 15:21                                                                 ` Joel Brobecker
2010-06-22 18:05                                                                 ` Eli Zaretskii
2010-06-23  2:03                                                                   ` Hui Zhu
2010-06-23 16:59                                                                     ` Joel Brobecker
2010-06-24  6:13                                                                       ` Hui Zhu
2010-01-06 10:13                                             ` Eli Zaretskii

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