Re: Sim hangs on new target at dup_arg_p() in infinite loop.

[Alexey Makhalov <makhaloff@gmail.com>]

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

fixed


Index: sim/common/sim-options.c
===================================================================
RCS file: /cvs/src/src/sim/common/sim-options.c,v
retrieving revision 1.31
diff -u -p -r1.31 sim-options.c
--- sim/common/sim-options.c 3 Sep 2013 20:45:08 -0000 1.31
+++ sim/common/sim-options.c 21 Sep 2013 02:56:18 -0000
@@ -35,6 +35,7 @@ along with this program.  If not, see <h
 #include "sim-assert.h"

 #include "bfd.h"
+#include "hashtab.h"

 /* Add a set of options to the simulator.
    TABLE is an array of OPTIONS terminated by a NULL `opt.name' entry.
@@ -485,37 +486,39 @@ standard_install (SIM_DESC sd)
   return SIM_RC_OK;
 }

+/* A simple comparison function with opposite semantics to strcmp.  */
+
+static int
+streq (const void *lhs, const void *rhs)
+{
+  return !strcmp ((const char *) lhs, (const char *) rhs);
+}
+
 /* Return non-zero if arg is a duplicate argument.
    If ARG is NULL, initialize.  */

-#define ARG_HASH_SIZE 97
-#define ARG_HASH(a) ((256 * (unsigned char) a[0] + (unsigned char)
a[1]) % ARG_HASH_SIZE)
+#define ARG_HASH_SIZE 256

 static int
 dup_arg_p (const char *arg)
 {
-  int hash;
-  static const char **arg_table = NULL;
+  void **slot;
+  static htab_t arg_table = NULL;

   if (arg == NULL)
     {
       if (arg_table == NULL)
- arg_table = (const char **) xmalloc (ARG_HASH_SIZE * sizeof (char *));
-      memset (arg_table, 0, ARG_HASH_SIZE * sizeof (char *));
+        arg_table = htab_create (ARG_HASH_SIZE, htab_hash_string, streq, free);
+      else
+ htab_empty (arg_table);
       return 0;
     }

-  hash = ARG_HASH (arg);
-  while (arg_table[hash] != NULL)
-    {
-      if (strcmp (arg, arg_table[hash]) == 0)
- return 1;
-      /* We assume there won't be more than ARG_HASH_SIZE arguments so we
- don't check if the table is full.  */
-      if (++hash == ARG_HASH_SIZE)
- hash = 0;
-    }
-  arg_table[hash] = arg;
+  slot = htab_find_slot (arg_table, arg, INSERT);
+  if (*slot != NULL)
+    return 1;
+
+  *slot = strdup (arg);
   return 0;
 }


On Fri, Sep 20, 2013 at 10:33 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Alexey" == Alexey Makhalov <makhaloff@gmail.com> writes:
>
> Alexey> +        arg_table = htab_create (ARG_HASH_SIZE, htab_hash_string,
> Alexey> +               (int (*)(const void *, const void *)) streq, free);
>
> I think casts like this are better avoided.
> Since streq is only used here, it is no trouble to just give it the
> proper type.
>
> Alexey> +  *slot = strdup(arg);
>
> The GNU coding style requires a space before the paren here.
>
> Tom

[-- Attachment #2: htab.patch --]
[-- Type: application/octet-stream, Size: 2023 bytes --]

Index: sim/common/sim-options.c
===================================================================
RCS file: /cvs/src/src/sim/common/sim-options.c,v
retrieving revision 1.31
diff -u -p -r1.31 sim-options.c
--- sim/common/sim-options.c	3 Sep 2013 20:45:08 -0000	1.31
+++ sim/common/sim-options.c	21 Sep 2013 02:56:18 -0000
@@ -35,6 +35,7 @@ along with this program.  If not, see <h
 #include "sim-assert.h"
 
 #include "bfd.h"
+#include "hashtab.h"
 
 /* Add a set of options to the simulator.
    TABLE is an array of OPTIONS terminated by a NULL `opt.name' entry.
@@ -485,37 +486,39 @@ standard_install (SIM_DESC sd)
   return SIM_RC_OK;
 }
 
+/* A simple comparison function with opposite semantics to strcmp.  */
+
+static int
+streq (const void *lhs, const void *rhs)
+{
+  return !strcmp ((const char *) lhs, (const char *) rhs);
+}
+
 /* Return non-zero if arg is a duplicate argument.
    If ARG is NULL, initialize.  */
 
-#define ARG_HASH_SIZE 97
-#define ARG_HASH(a) ((256 * (unsigned char) a[0] + (unsigned char) a[1]) % ARG_HASH_SIZE)
+#define ARG_HASH_SIZE 256
 
 static int
 dup_arg_p (const char *arg)
 {
-  int hash;
-  static const char **arg_table = NULL;
+  void **slot;
+  static htab_t arg_table = NULL;
 
   if (arg == NULL)
     {
       if (arg_table == NULL)
-	arg_table = (const char **) xmalloc (ARG_HASH_SIZE * sizeof (char *));
-      memset (arg_table, 0, ARG_HASH_SIZE * sizeof (char *));
+        arg_table = htab_create (ARG_HASH_SIZE, htab_hash_string, streq, free);
+      else
+	htab_empty (arg_table);
       return 0;
     }
 
-  hash = ARG_HASH (arg);
-  while (arg_table[hash] != NULL)
-    {
-      if (strcmp (arg, arg_table[hash]) == 0)
-	return 1;
-      /* We assume there won't be more than ARG_HASH_SIZE arguments so we
-	 don't check if the table is full.  */
-      if (++hash == ARG_HASH_SIZE)
-	hash = 0;
-    }
-  arg_table[hash] = arg;
+  slot = htab_find_slot (arg_table, arg, INSERT);
+  if (*slot != NULL)
+    return 1;
+
+  *slot = strdup (arg);
   return 0;
 }