RE: "set record instruction-history" ?

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Tuesday, March 26, 2013 7:07 PM
> To: Metzger, Markus T


> Thanks!  Taking into account the push back in making the
> set history size command signed, I've adjusted the patch
> to keep this command signed as well and checked it in.
> I tried the different numbers that should be affected manually
> and I believe it to be an equivalent patch from what you tested.
> Let me know, if something broke.

There is a single regression...

[...]

> +static int
> +command_size_to_target_size (unsigned int *command)
> +{
> +  gdb_assert (*command <= INT_MAX || *command == UINT_MAX);
> +
> +  if (record_call_history_size == UINT_MAX)

...here. We should have used *command in the comparison.

I also changed the parameter type to "unsigned int" since we're not
updating our argument.

    record: fix instruction-history-size regression
    
    (gdb) PASS: gdb.btrace/instruction_history.exp: set record instruction-history-size 0
    record instruction-history 0
    Bad range.
    (gdb) FAIL: gdb.btrace/instruction_history.exp: record instruction-history - unlimited
    
    2013-03-13  Markus Metzger  <markus.t.metzger@intel.com>
    
    	* record.c (command_size_to_target_size): Fix size comparison.
    	Change parameter type from pointer to integer to integer.
    	Update all users.
    
diff --git a/gdb/record.c b/gdb/record.c
index b64f3bc..2736a1e 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -442,18 +442,18 @@ get_insn_history_modifiers (char **arg)
    meaning unlimited.  The target interfaces works with signed int
    though, to indicate direction, so map "unlimited" to INT_MAX, which
    is about the same as unlimited in practice.  If the user does have
-   a log that huge, she can can fetch it in chunks across several
-   requests, but she'll likely have other problems first...  */
+   a log that huge, she can fetch it in chunks across several requests,
+   but she'll likely have other problems first...  */
 
 static int
-command_size_to_target_size (unsigned int *command)
+command_size_to_target_size (unsigned int size)
 {
-  gdb_assert (*command <= INT_MAX || *command == UINT_MAX);
+  gdb_assert (size <= INT_MAX || size == UINT_MAX);
 
-  if (record_call_history_size == UINT_MAX)
+  if (size == UINT_MAX)
     return INT_MAX;
   else
-    return *command;
+    return size;
 }
 
 /* The "record instruction-history" command.  */
@@ -467,7 +467,7 @@ cmd_record_insn_history (char *arg, int from_tty)
 
   flags = get_insn_history_modifiers (&arg);
 
-  size = command_size_to_target_size (&record_insn_history_size);
+  size = command_size_to_target_size (record_insn_history_size);
 
   if (arg == NULL || *arg == 0 || strcmp (arg, "+") == 0)
     target_insn_history (size, flags);
@@ -583,7 +583,7 @@ cmd_record_call_history (char *arg, int from_tty)
 
   flags = get_call_history_modifiers (&arg);
 
-  size = command_size_to_target_size (&record_call_history_size);
+  size = command_size_to_target_size (record_call_history_size);
 
   if (arg == NULL || *arg == 0 || strcmp (arg, "+") == 0)
     target_call_history (size, flags);


OK to commit?  Does this also go onto the 7.6 branch?

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052