Have `scm_take_locale_symbol ()' return an interned symbol (fixes bug #25865).

* libguile/symbols.c (intern_symbol): New function, with code formerly
  duplicated in `scm_i_c_mem2symbol ()' and `scm_i_mem2symbol ()'.
  (scm_i_c_mem2symbol, scm_i_mem2symbol): Use it.
  (scm_take_locale_symboln): Use `intern_symbol ()'.  This fixes
  bug #25865.

* test-suite/standalone/Makefile.am
  (test_scm_take_locale_symbol_SOURCES,
  test_scm_take_locale_symbol_CFLAGS,
  test_scm_take_locale_symbol_LDADD): New variables.
  (check_PROGRAMS, TESTS): Add `test-scm-take-locale-symbol'.

NEWS

@@ -64,6 +64,8 @@ transformed by (ice-9 syncase) would cause an "Invalid syntax" error.
 Now it works as you would expect (giving the value of the specified
 module binding).
 
+** Have `scm_take_locale_symbol ()' return an interned symbol (bug #25865)
+
 
 Changes in 1.8.6 (since 1.8.5)
 

libguile/symbols.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,2000,2001, 2003, 2004, 2006 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,2000,2001, 2003, 2004, 2006, 2009 Free Software Foundation, Inc.
  * 
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -122,31 +122,40 @@ lookup_interned_symbol (const char *name, size_t len,
   return SCM_BOOL_F;
 }
 
+/* Intern SYMBOL, an uninterned symbol.  */
+static void
+intern_symbol (SCM symbol)
+{
+  SCM slot, cell;
+  unsigned long hash;
+
+  hash = scm_i_symbol_hash (symbol) % SCM_HASHTABLE_N_BUCKETS (symbols);
+  slot = SCM_HASHTABLE_BUCKET (symbols, hash);
+  cell = scm_cons (symbol, SCM_UNDEFINED);
+
+  SCM_SET_HASHTABLE_BUCKET (symbols, hash, scm_cons (cell, slot));
+  SCM_HASHTABLE_INCREMENT (symbols);
+
+  if (SCM_HASHTABLE_N_ITEMS (symbols) > SCM_HASHTABLE_UPPER (symbols))
+    scm_i_rehash (symbols, scm_i_hash_symbol, 0, "intern_symbol");
+}
+
 static SCM
 scm_i_c_mem2symbol (const char *name, size_t len)
 {
   SCM symbol;
   size_t raw_hash = scm_string_hash ((const unsigned char *) name, len);
-  size_t hash = raw_hash % SCM_HASHTABLE_N_BUCKETS (symbols);
 
   symbol = lookup_interned_symbol (name, len, raw_hash);
-  if (symbol != SCM_BOOL_F)
-    return symbol;
+  if (scm_is_false (symbol))
+    {
+      /* The symbol was not found, create it.  */
+      symbol = scm_i_c_make_symbol (name, len, 0, raw_hash,
+				    scm_cons (SCM_BOOL_F, SCM_EOL));
+      intern_symbol (symbol);
+    }
 
-  {
-    /* The symbol was not found - create it. */
-    SCM symbol = scm_i_c_make_symbol (name, len, 0, raw_hash,
-				      scm_cons (SCM_BOOL_F, SCM_EOL));
-
-    SCM slot = SCM_HASHTABLE_BUCKET (symbols, hash);
-    SCM cell = scm_cons (symbol, SCM_UNDEFINED);
-    SCM_SET_HASHTABLE_BUCKET (symbols, hash, scm_cons (cell, slot));
-    SCM_HASHTABLE_INCREMENT (symbols);
-    if (SCM_HASHTABLE_N_ITEMS (symbols) > SCM_HASHTABLE_UPPER (symbols))
-      scm_i_rehash (symbols, scm_i_hash_symbol, 0, "scm_mem2symbol");
-
-    return symbol;
-  }
+  return symbol;
 }
 
 static SCM
@@ -156,26 +165,17 @@ scm_i_mem2symbol (SCM str)
   const char *name = scm_i_string_chars (str);
   size_t len = scm_i_string_length (str);
   size_t raw_hash = scm_string_hash ((const unsigned char *) name, len);
-  size_t hash = raw_hash % SCM_HASHTABLE_N_BUCKETS (symbols);
 
   symbol = lookup_interned_symbol (name, len, raw_hash);
-  if (symbol != SCM_BOOL_F)
-    return symbol;
+  if (scm_is_false (symbol))
+    {
+      /* The symbol was not found, create it.  */
+      symbol = scm_i_make_symbol (str, 0, raw_hash,
+				  scm_cons (SCM_BOOL_F, SCM_EOL));
+      intern_symbol (symbol);
+    }
 
-  {
-    /* The symbol was not found - create it. */
-    SCM symbol = scm_i_make_symbol (str, 0, raw_hash,
-				    scm_cons (SCM_BOOL_F, SCM_EOL));
-
-    SCM slot = SCM_HASHTABLE_BUCKET (symbols, hash);
-    SCM cell = scm_cons (symbol, SCM_UNDEFINED);
-    SCM_SET_HASHTABLE_BUCKET (symbols, hash, scm_cons (cell, slot));
-    SCM_HASHTABLE_INCREMENT (symbols);
-    if (SCM_HASHTABLE_N_ITEMS (symbols) > SCM_HASHTABLE_UPPER (symbols))
-      scm_i_rehash (symbols, scm_i_hash_symbol, 0, "scm_mem2symbol");
-
-    return symbol;
-  }
+  return symbol;
 }
 
 
@@ -416,14 +416,14 @@ scm_take_locale_symboln (char *sym, size_t len)
 
   raw_hash = scm_string_hash ((unsigned char *)sym, len);
   res = lookup_interned_symbol (sym, len, raw_hash);
-  if (res != SCM_BOOL_F)
+  if (scm_is_false (res))
     {
-      free (sym);
-      return res;
+      res = scm_i_c_take_symbol (sym, len, 0, raw_hash,
+				 scm_cons (SCM_BOOL_F, SCM_EOL));
+      intern_symbol (res);
     }
-
-  res = scm_i_c_take_symbol (sym, len, 0, raw_hash,
-			     scm_cons (SCM_BOOL_F, SCM_EOL));
+  else
+    free (sym);
 
   return res;
 }

test-suite/standalone/.gitignore

@@ -9,3 +9,4 @@
 /test-scm-with-guile
 /test-scm-c-read
 /test-fast-slot-ref
+/test-scm-take-locale-symbol

test-suite/standalone/Makefile.am

@@ -1,6 +1,6 @@
 ## Process this file with automake to produce Makefile.in.
 ##
-## Copyright 2003, 2004, 2005, 2006, 2007, 2008 Software Foundation, Inc.
+## Copyright 2003, 2004, 2005, 2006, 2007, 2008, 2009 Software Foundation, Inc.
 ##
 ## This file is part of GUILE.
 ##
@@ -118,6 +118,14 @@ test_scm_c_read_LDADD = ${top_builddir}/libguile/libguile.la
 check_PROGRAMS += test-scm-c-read
 TESTS += test-scm-c-read
 
+# test-scm-take-locale-symbol
+test_scm_take_locale_symbol_SOURCES = test-scm-take-locale-symbol.c
+test_scm_take_locale_symbol_CFLAGS = ${test_cflags}
+test_scm_take_locale_symbol_LDADD = ${top_builddir}/libguile/libguile.la
+check_PROGRAMS += test-scm-take-locale-symbol
+TESTS += test-scm-take-locale-symbol
+
+
 if BUILD_PTHREAD_SUPPORT
 
 # test-with-guile-module

test-suite/standalone/test-scm-take-locale-symbol.c

@@ -0,0 +1,63 @@
+/* Copyright (C) 2009 Free Software Foundation, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Exercise `scm_take_locale_symbol ()', making sure it returns an interned
+   symbol.  See https://savannah.gnu.org/bugs/index.php?25865 .  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <libguile.h>
+
+#include <stdlib.h>
+#include <string.h>
+
+
+static void *
+do_test (void *result)
+{
+  SCM taken_sym, sym;
+
+  taken_sym = scm_take_locale_symbol (strdup ("some random symbol"));
+  sym = scm_from_locale_symbol ("some random symbol");
+
+  if (scm_is_true (scm_symbol_p (sym))
+      && scm_is_true (scm_symbol_p (taken_sym))
+
+      /* Relying solely on `scm_symbol_interned_p ()' is insufficient since
+	 it doesn't reflect the actual state of the symbol hashtable, hence
+	 the additional `scm_is_eq' test.  */
+      && scm_is_true (scm_symbol_interned_p (sym))
+      && scm_is_true (scm_symbol_interned_p (taken_sym))
+      && scm_is_eq (taken_sym, sym))
+    * (int *) result = EXIT_SUCCESS;
+  else
+    * (int *) result = EXIT_FAILURE;
+
+  return NULL;
+}
+
+int
+main (int argc, char *argv[])
+{
+  int result;
+
+  scm_with_guile (do_test, &result);
+
+  return result;
+}