[12/18] Implement completion limiting for sim_command_completer.

Message ID 20150413192331.29172.23243.stgit@valrhona.uglyboxes.com
State Superseded
Headers

Commit Message

Keith Seitz April 13, 2015, 7:23 p.m. UTC
  This patch converts sim_command_completer to use maybe_add_completion.
It does not add any tests, since the `sim' command is highly
target-dependent and unimplemented for the majority of simulators.

gdb/ChangeLog

	* remote-sim.c: Include completer.h.
	(sim_command_completer): Use maybe_add_completion.
---
 gdb/remote-sim.c |   28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)
  

Comments

Mike Frysinger April 14, 2015, 3:28 p.m. UTC | #1
On 13 Apr 2015 12:23, Keith Seitz wrote:
> This patch converts sim_command_completer to use maybe_add_completion.

i think you want to push this down into sim/common/sim-options.c ?  otherwise 
you do all of this work only to throw away a good amount of it.

> It does not add any tests, since the `sim' command is highly
> target-dependent and unimplemented for the majority of simulators.

erm, the majority of simulators now have testsuites.  but that doesn't really 
matter because you're updating the gdb part of the API, not the sim side.
-mike
  
Keith Seitz May 5, 2015, 3:03 p.m. UTC | #2
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/14/2015 08:28 AM, Mike Frysinger wrote:
> i think you want to push this down into sim/common/sim-options.c ?
> otherwise you do all of this work only to throw away a good amount
> of it.

Eventually, yes, but not right now, if I may. I'd be more than happy
to prepare and submit a follow-up patch for sim/ in the next week or
two (pending acceptance of this patch series).

>> It does not add any tests, since the `sim' command is highly 
>> target-dependent and unimplemented for the majority of
>> simulators.
> 
> erm, the majority of simulators now have testsuites.  but that
> doesn't really matter because you're updating the gdb part of the
> API, not the sim side.

Right, and I can only test it on any sim-enabled target, which I
seldom use anymore.

Keith

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVSNvBAAoJEM4/L8mVMm6zV/oQAKAy+9SyBGEXxI8FVQUyhRlL
y0Hi9ZHa5aTO+pyn0aDNkhJMSyS53F3nNLt2IEvgr/+blN+YISufpU6Tn4AX59gD
1lTzGXMKHRhdZ/uo5x+v0jM/i+05cZV72bfyvWydt+zJlU6mxV1vTwMrxIJRBgE8
ibbzvXNCCll1tkzICAe/fFVOOLgQpbSKj2PQH3f1RhwX7Xa0K0oGfxQhsh8iGVue
SGVdIS0H1EMeegcHT5IxasykqUD1IMKFVCv/pdvFJQU+2F8bqlNPMT0ODabULHLq
i6gLEicLMG4XfFjs7yOttQqezrCWk5fR78IH/fFn/xex8e58KX5F7cyVCo2qf3hC
1zJjw0+N2tpDSBXmos8s064+pFWK4dbFin36ASTpMFI7Qf/QqgQhSRYbgQ3zvXw7
pLzC4tkdCBJ/2IKsLYQZf3zr8S8wKKCG5jJkXeThbhFXMz1ZviQZ/evTkGcbLSi2
K8PBKilWYwPktL6mN1cQB0UDi7g9lvYb5v7Q+oM2boCH7LLNgKiGTrQC6sCh+CLK
emz6EPiSt0tiOaDQo1txcOL3jKEojZyoAwGuIFohtvrHmWYsDr5DAz96ZiWvTirE
IrztQ5toVTKo1rQ8MtKjwcCi0rGDOhJ/FEyvZN7aZ8NPW8IdyaJcp6Ozrjq1ZLl+
yaamOkqLO4CsBsK5rUNr
=Gh9i
-----END PGP SIGNATURE-----
  
Mike Frysinger May 5, 2015, 3:34 p.m. UTC | #3
On 05 May 2015 08:03, Keith Seitz wrote:
> On 04/14/2015 08:28 AM, Mike Frysinger wrote:
> > i think you want to push this down into sim/common/sim-options.c ?
> > otherwise you do all of this work only to throw away a good amount
> > of it.
> 
> Eventually, yes, but not right now, if I may. I'd be more than happy
> to prepare and submit a follow-up patch for sim/ in the next week or
> two (pending acceptance of this patch series).
> 
> >> It does not add any tests, since the `sim' command is highly 
> >> target-dependent and unimplemented for the majority of
> >> simulators.
> > 
> > erm, the majority of simulators now have testsuites.  but that
> > doesn't really matter because you're updating the gdb part of the
> > API, not the sim side.
> 
> Right, and I can only test it on any sim-enabled target, which I
> seldom use anymore.

that's pretty trivial to get though

$ mkdir build
$ cd build
$ ../configure --target=ft32-elf
$ make all-gdb
$ ./gdb/gdb
(gdb) target sim
(gdb) sim <tab>
-mike
  
Keith Seitz May 5, 2015, 3:53 p.m. UTC | #4
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 05/05/2015 08:34 AM, Mike Frysinger wrote:
> that's pretty trivial to get though
> 
> $ mkdir build $ cd build $ ../configure --target=ft32-elf $ make
> all-gdb $ ./gdb/gdb (gdb) target sim (gdb) sim <tab>

Sure, and thanks to you [:-)] I did a nearly trivial amount of digging
and found what *might* be a reliable way to test if the test suite is
using a simulator target. mi-support.exp's mi_gdb_target_load tests
"target_info protocol" for "sim".

So looks like I might actually be able to generically write a test for
this.

Thanks for the prodding!

Keith

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJVSOdxAAoJEM4/L8mVMm6z/+sP/RfHfZi3Ftw+jJfjmJri7xM6
i2nS5kVlvCd6tCxLIEIW9d8epHqkMK75UsclV7SLuiwjHwh3n4dZUxCP6LpgGLbM
ww2nL46qk3KAzVLI+phIHqso1lZRVbp2cWrS1ipEbfoaRwig3dfeWJX3WLa0pJSa
+tw6rdTZ6kKFqqGh0kTrvVwG3gQNg9vFJ1t/7y10Eug7uswc7SVKA6oxGEuPPspo
R+6GJOkkfL4CfJh9HqztPCd4FdEnG6nn2FILpcVuXeXdVVMw0ls/AMTIltMkXB/V
4pTrtIBQm3mu5jC4NyL+RXdQ+f8u3rqBv9AOm7WRAAF658ic1qqUPcpVUYOPwvOd
xlFg3Wz7f8QZSn6+I8t2bx7YTbdWTEUoQN7h7HQBoFKyZfrs938Rwf2Y+b0udqyi
R8jnaWLEsOfCeZL6qH29O7UUwDsAIL6URMZaddQRHk4q48sO34+7k2gXQnDBMZgw
zzb093al0xPKKD7Qb4xJ4UqG2YGygOScPJTuupiszFLuL8TdQFKFZjDL46XeuD82
RiGd9+XY6hT8nt3zgDReAAy2EoiXq7UASkz87AI1UFzJeV5fanqJKhKIUUeI7+eh
eCLo5lYnNweHoKm1NnvajH8Hm+KmwRT87QgN5W+YJmV8lddBqRXsXI+H6vGYGXe2
ifz5rM+n49gHe6XqAAz5
=+5N5
-----END PGP SIGNATURE-----
  

Patch

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index 2bcb19e..5056a13 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -39,6 +39,7 @@ 
 #include "arch-utils.h"
 #include "readline/readline.h"
 #include "gdbthread.h"
+#include "completer.h"
 
 /* Prototypes */
 
@@ -1223,7 +1224,7 @@  sim_command_completer (struct completer_data *cdata,
 {
   struct sim_inferior_data *sim_data;
   char **tmp;
-  int i;
+  int i, stop;
   VEC (char_ptr) *result = NULL;
 
   sim_data = inferior_data (current_inferior (), sim_inferior_data_key);
@@ -1235,8 +1236,29 @@  sim_command_completer (struct completer_data *cdata,
     return NULL;
 
   /* Transform the array into a VEC, and then free the array.  */
-  for (i = 0; tmp[i] != NULL; i++)
-    VEC_safe_push (char_ptr, result, tmp[i]);
+  for (i = 0, stop = 0; !stop && tmp[i] != NULL; i++)
+    {
+      enum maybe_add_completion_enum add_status;
+
+      add_status = maybe_add_completion (cdata, tmp[i]);
+      switch (add_status)
+	{
+	case MAYBE_ADD_COMPLETION_OK:
+	  VEC_safe_push (char_ptr, result, tmp[i]);
+	  break;
+	case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
+	  VEC_safe_push (char_ptr, result, tmp[i]);
+	  stop = 1;
+	  break;
+	case MAYBE_ADD_COMPLETION_MAX_REACHED:
+	  xfree (tmp[i]);
+	  stop = 1;
+	  break;
+	case MAYBE_ADD_COMPLETION_DUPLICATE:
+	  xfree (tmp[i]);
+	  break;
+	}
+    }
   xfree (tmp);
 
   return result;