[PATCH v2 0/4] Some fixes to libgst

Hi,

I have accumulated some fixes to libgst.  You may want to take a look.

This version 2 have no change since v1, only rebased to top of master
branch.

Regards,
lee

Lee Duhem (4):
  libgst: Fix comments
  libgst: Miscellaneous improving to code style
  libgst: Some trivial fixes
  libgst: Define and use COUNT_OF

 libgst/ChangeLog | 38 ++++++++++++++++++++++++++++++++++++++
 libgst/byte.h    |  2 +-
 libgst/callin.c  |  7 +++++--
 libgst/cint.c    |  7 +++++--
 libgst/dict.c    | 34 +++++++++++++++-------------------
 libgst/dict.h    |  8 ++++----
 libgst/files.c   |  4 ++--
 libgst/gst.h     |  2 +-
 libgst/gstpriv.h |  5 +++++
 libgst/oop.c     |  2 +-
 libgst/oop.h     |  9 +++------
 libgst/save.c    |  2 +-
 libgst/sym.c     |  6 +++---
 libgst/sysdep.h  |  2 +-
 14 files changed, 85 insertions(+), 43 deletions(-)

--
2.13.5


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

2017-04-26  Lee Duhem  <[hidden email]>

       * byte.h: Fix typo in comments.
       * dict.h: Improve comments.
       * dict.c (new_num_fields): Ditto.
       * gst.h: Ditto.
       * sysdep.h: Ditto.
---
 libgst/ChangeLog | 8 ++++++++
 libgst/byte.h    | 2 +-
 libgst/dict.c    | 4 ++--
 libgst/dict.h    | 8 ++++----
 libgst/gst.h     | 2 +-
 libgst/oop.h     | 5 +----
 libgst/sysdep.h  | 2 +-
 7 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 5daf52ba..d8e413c7 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,3 +1,11 @@
+2017-04-26  Lee Duhem  <[hidden email]>
+
+ * byte.h: Fix typo in comments.
+ * dict.h: Improve comments.
+ * dict.c (new_num_fields): Ditto.
+ * gst.h: Ditto.
+ * sysdep.h: Ditto.
+
 2016-12-30  Lee Duhem  <[hidden email]>
 
  * byte.c (_gst_compile_bytecodes): Subtract the free space.
diff --git a/libgst/byte.h b/libgst/byte.h
index 08e8bcba..78ecd9e8 100644
--- a/libgst/byte.h
+++ b/libgst/byte.h
@@ -131,7 +131,7 @@ enum {
   LN_FORCE = 1,
 
   /* If LN_ABSOLUTE is also set, causes _gst_line_number to emit an absolute
-     ine number and use that line number as the offset.  If not,
+     line number and use that line number as the offset.  If not,
      _gst_line_number will emit line numbers relatives to that line.  */
   LN_RESET = 2,
 
diff --git a/libgst/dict.c b/libgst/dict.c
index 8b37f0fe..517bd5dc 100644
--- a/libgst/dict.c
+++ b/libgst/dict.c
@@ -197,7 +197,7 @@ static size_t new_num_fields (size_t oldNumFields);
 
 /* Instantiate the OOPs that are created before the first classes
    (true, false, nil, the Smalltalk dictionary, the symbol table
-   and Processor, the sole instance of ProcessorScheduler.  */
+   and Processor, the sole instance of ProcessorScheduler).  */
 static void init_proto_oops (void);
 
 /* Look for the index at which KEYOOP resides in IDENTITYDICTIONARYOOP
@@ -211,7 +211,7 @@ static ssize_t identity_dictionary_find_key (OOP identityDictionaryOOP,
 static size_t identity_dictionary_find_key_or_nil (OOP identityDictionaryOOP,
    OOP keyOOP);
 
-/* assume the value is an integer already or key does not exist, increase the
+/* Assume the value is an integer already or key does not exist, increase the
    value by inc or set the value to inc */
 static int _gst_identity_dictionary_at_inc (OOP identityDictionaryOOP,
                                             OOP keyOOP,
diff --git a/libgst/dict.h b/libgst/dict.h
index 5aaa7eba..303cc565 100644
--- a/libgst/dict.h
+++ b/libgst/dict.h
@@ -468,14 +468,14 @@ extern OOP _gst_class_variable_dictionary (OOP class_oop)
   ATTRIBUTE_PURE
   ATTRIBUTE_HIDDEN;
 
-/* This finds the key SYMBOL into the dictionary POOLOOP and, if any,
+/* This finds the key SYMBOL in the dictionary POOLOOP and, if any,
    in all of its super-namespaces.  Returns the association.  */
 extern OOP _gst_namespace_association_at (OOP poolOOP,
   OOP symbol)
   ATTRIBUTE_PURE
   ATTRIBUTE_HIDDEN;
 
-/* This finds the key SYMBOL into the dictionary POOLOOP and, if any,
+/* This finds the key SYMBOL in the dictionary POOLOOP and, if any,
    in all of its super-namespaces.  Returns the value.  */
 extern OOP _gst_namespace_at (OOP poolOOP,
       OOP symbol)
@@ -517,7 +517,7 @@ extern OOP _gst_find_class (OOP classNameOOP)
   ATTRIBUTE_PURE
   ATTRIBUTE_HIDDEN;
 
-/* Look for an implementation of SELECTOR (a Symbol) into CLASS_OOP's
+/* Look for an implementation of SELECTOR (a Symbol) in CLASS_OOP's
    method dictionary or in the method dictionary of a superclass.  */
 extern OOP _gst_find_class_method (OOP class_oop,
    OOP selector)
@@ -647,7 +647,7 @@ extern void _gst_free_cobject (OOP cObjOOP)
   ATTRIBUTE_HIDDEN;
 
 /* Loads the contents of the global variables from the Smalltalk dictionary
-   after an image has been restored.  PRIM_TABLE_MATCHES if true if the
+   after an image has been restored.  PRIM_TABLE_MATCHES is true if the
    table of primitives is already set up correctly.  */
 extern mst_Boolean _gst_init_dictionary_on_image_load (mst_Boolean prim_table_matches)
   ATTRIBUTE_HIDDEN;
diff --git a/libgst/gst.h b/libgst/gst.h
index da816180..a6906e91 100644
--- a/libgst/gst.h
+++ b/libgst/gst.h
@@ -176,7 +176,7 @@ struct object_s
 #define IS_OOP(oop) \
   (! IS_INT(oop) )
 
-/* Keep these in sync with _gst_sizes, in dict.c.
+/* Keep these in sync with _gst_log2_sizes, in dict.c.
    FIXME: these should be exported in a pool dictionary.  */
 enum gst_indexed_kind {
   GST_ISP_FIXED = 0,
diff --git a/libgst/oop.h b/libgst/oop.h
index 76a5977d..efc51da0 100644
--- a/libgst/oop.h
+++ b/libgst/oop.h
@@ -412,10 +412,7 @@ extern gst_object _gst_alloc_words (size_t size)
   ATTRIBUTE_HIDDEN;
 
 /* Grows the allocated memory to SPACESIZE bytes, if it's not there
-   already.  
-   the memory could not be allocated.  Should be called after the
-   sweep has occurred so that things are contiguous.  Ensures that the
-   OOP table pointers are fixed up to point to the new objects.  */
+   already. */
 extern void _gst_grow_memory_to (size_t size)
   ATTRIBUTE_HIDDEN;
 
diff --git a/libgst/sysdep.h b/libgst/sysdep.h
index 5648ef28..65e7e979 100644
--- a/libgst/sysdep.h
+++ b/libgst/sysdep.h
@@ -153,7 +153,7 @@ extern uint64_t _gst_get_ns_time (void)
 extern time_t _gst_get_time (void)
   ATTRIBUTE_HIDDEN;
 
-/* Returns whether FILE1 is newer (or last modified at the same time as)
+/* Returns whether FILE1 is newer than (or last modified at the same time as)
    FILE2.  Returns true if FILE2 is not readable, false if FILE1 is not
    readable.  */
 extern mst_Boolean _gst_file_is_newer (const char *file1, const char *file2)
--
2.13.5


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

2017-04-26  Lee Duhem  <[hidden email]>

      * cint.c: Include ffi.h properly.
      * dict.c (create_classes_pass1): Use INCR_INT.
      (init_c_symbols): Replace magic number with corresponding macro.
      (new_num_fields): Move variable assignment closer to its use.
      (find_key_or_nil): Ditto.
      (_gst_to_wide_cstring): Replace magic number with meaningful
      expression.
      * files.c (_gst_initialize): Simplify code.
      * save.c (save_object): Improve readability of magic number.
---
 libgst/ChangeLog | 12 ++++++++++++
 libgst/cint.c    |  2 +-
 libgst/dict.c    | 14 ++++++--------
 libgst/files.c   |  4 ++--
 libgst/save.c    |  2 +-
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index d8e413c7..e876657d 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,17 @@
 2017-04-26  Lee Duhem  <[hidden email]>
 
+ * cint.c: Include ffi.h properly.
+ * dict.c (create_classes_pass1): Use INCR_INT.
+ (init_c_symbols): Replace magic number with corresponding macro.
+ (new_num_fields): Move variable assignment closer to its use.
+ (find_key_or_nil): Ditto.
+ (_gst_to_wide_cstring): Replace magic number with meaningful
+ expression.
+ * files.c (_gst_initialize): Simplify code.
+ * save.c (save_object): Improve readability of magic number.
+
+2017-04-26  Lee Duhem  <[hidden email]>
+
  * byte.h: Fix typo in comments.
  * dict.h: Improve comments.
  * dict.c (new_num_fields): Ditto.
diff --git a/libgst/cint.c b/libgst/cint.c
index fdb6ce39..6848234a 100644
--- a/libgst/cint.c
+++ b/libgst/cint.c
@@ -56,7 +56,7 @@
 #include "re.h"
 #include "pointer-set.h"
 
-#include "ffi.h"
+#include <ffi.h>
 #include <ltdl.h>
 
 #ifdef HAVE_GETGRNAM
diff --git a/libgst/dict.c b/libgst/dict.c
index 517bd5dc..9acb44cc 100644
--- a/libgst/dict.c
+++ b/libgst/dict.c
@@ -858,8 +858,7 @@ create_classes_pass1 (const class_definition *ci,
       else
  {
           superclass = (gst_class) OOP_TO_OBJ (superClassOOP);
-          superclass->subClasses =
-    FROM_INT (TO_INT (superclass->subClasses) + 1);
+          superclass->subClasses = INCR_INT (superclass->subClasses);
  }
     }
 
@@ -1225,7 +1224,7 @@ init_c_symbols ()
 void
 init_primitives_dictionary ()
 {
-  OOP primDictionaryOOP = _gst_binding_dictionary_new (512, _gst_smalltalk_dictionary);
+  OOP primDictionaryOOP = _gst_binding_dictionary_new (NUM_PRIMITIVES, _gst_smalltalk_dictionary);
   int i;
 
   add_smalltalk ("VMPrimitives", primDictionaryOOP);
@@ -1517,7 +1516,7 @@ new_num_fields (size_t oldNumFields)
 {
   /* Find a power of two that is larger than oldNumFields */
 
-  int n = 1;
+  int n;
 
   /* Already a power of two? duplicate the size */
   if COMMON ((oldNumFields & (oldNumFields - 1)) == 0)
@@ -1525,7 +1524,7 @@ new_num_fields (size_t oldNumFields)
 
   /* Find the next power of two by setting all bits to the right of
      the leftmost 1 bit to 1, and then incrementing.  */
-  for (; oldNumFields & (oldNumFields + 1); n <<= 1)
+  for (n = 1; oldNumFields & (oldNumFields + 1); n <<= 1)
     oldNumFields |= oldNumFields >> n;
 
   return oldNumFields + 1;
@@ -1545,9 +1544,8 @@ find_key_or_nil (OOP dictionaryOOP,
   numFixedFields = OOP_FIXED_FIELDS (dictionaryOOP);
   numFields = NUM_WORDS (dictionary) - numFixedFields;
   index = scramble (OOP_INDEX (keyOOP));
-  count = numFields;
 
-  for (; count; count--)
+  for (count = numFields; count; count--)
     {
       index &= numFields - 1;
       associationOOP = dictionary->data[numFixedFields + index];
@@ -2025,7 +2023,7 @@ _gst_to_wide_cstring (OOP stringOOP)
   string = (gst_unicode_string) OOP_TO_OBJ (stringOOP);
   len = oop_num_fields (stringOOP);
   result = (wchar_t *) xmalloc (len + 1);
-  if (sizeof (wchar_t) == 4)
+  if (sizeof (wchar_t) == sizeof(string->chars[0]))
     memcpy (result, string->chars, len * sizeof (wchar_t));
   else
     for (p = result, i = 0; i < len; i++)
diff --git a/libgst/files.c b/libgst/files.c
index 531c7fee..ee14ecfd 100644
--- a/libgst/files.c
+++ b/libgst/files.c
@@ -360,7 +360,7 @@ _gst_initialize (const char *kernel_dir,
   /* For the image file, it is okay to find none if we can/should rebuild
      the image file.  */
   if (image_file
-      && (flags & (GST_REBUILD_IMAGE | GST_MAYBE_REBUILD_IMAGE)) == 0
+      && !rebuild_image_flags
       && !_gst_file_is_readable (image_file))
     {
       if (flags & GST_IGNORE_BAD_IMAGE_PATH)
@@ -470,7 +470,7 @@ _gst_initialize (const char *kernel_dir,
       setvbuf (stdout, NULL, _IOLBF, 1024);
     }
 
-  if (rebuild_image_flags == 0)
+  if (!rebuild_image_flags)
     loadBinary = abortOnFailure = true;
   else
     {
diff --git a/libgst/save.c b/libgst/save.c
index be980a74..83e96926 100644
--- a/libgst/save.c
+++ b/libgst/save.c
@@ -419,7 +419,7 @@ save_object (int imageFd,
     abort ();
 
   numBytes = sizeof (OOP) * TO_INT (object->objSize);
-  if (numBytes < 262144)
+  if (numBytes < 256 * 1024)
     {
       saveObject = alloca (numBytes);
       fixup_object (oop, saveObject, object, numBytes);
--
2.13.5


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

2017-04-26  Lee Duhem  <[hidden email]>

       * callin.c (_gst_type_name_to_oop): Replace sprintf with snprintf.
       (_gst_class_name_to_oop): Clean properly before return.
       * cint.c (_gst_invoke_croutine): Clean properly before return.
       * dict.c (create_metaclass): Synchronize prototype with definition.
       (init_smalltalk_dictionary): Replace sprintf with snprintf.
       * oop.c (_gst_alloc_obj): Reduce size properly.
       * oop.h (INITIAL_OOP_TABLE_SIZE): Sychronize with comment.
       (_gst_grow_memory_to): Synchronize prototype with definition.
---
 libgst/ChangeLog | 11 +++++++++++
 libgst/callin.c  |  7 +++++--
 libgst/cint.c    |  5 ++++-
 libgst/dict.c    |  8 ++++----
 libgst/oop.c     |  2 +-
 libgst/oop.h     |  4 ++--
 6 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index e876657d..4d1375cc 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,16 @@
 2017-04-26  Lee Duhem  <[hidden email]>
 
+ * callin.c (_gst_type_name_to_oop): Replace sprintf with snprintf.
+ (_gst_class_name_to_oop): Clean properly before return.
+ * cint.c (_gst_invoke_croutine): Clean properly before return.
+ * dict.c (create_metaclass): Synchronize prototype with definition.
+ (init_smalltalk_dictionary): Replace sprintf with snprintf.
+ * oop.c (_gst_alloc_obj): Reduce size properly.
+ * oop.h (INITIAL_OOP_TABLE_SIZE): Sychronize with comment.
+ (_gst_grow_memory_to): Synchronize prototype with definition.
+
+2017-04-26  Lee Duhem  <[hidden email]>
+
  * cint.c: Include ffi.h properly.
  * dict.c (create_classes_pass1): Use INCR_INT.
  (init_c_symbols): Replace magic number with corresponding macro.
diff --git a/libgst/callin.c b/libgst/callin.c
index dd30511c..369e0bec 100644
--- a/libgst/callin.c
+++ b/libgst/callin.c
@@ -386,7 +386,7 @@ _gst_type_name_to_oop (const char *name)
   OOP result;
   char buf[300];
 
-  sprintf (buf, "^%s!", name);
+  snprintf (buf, sizeof (buf), "^%s!", name);
 
   result = _gst_eval_expr (buf);
   return (result);
@@ -469,7 +469,10 @@ _gst_class_name_to_oop (const char *name)
       key = _gst_intern_string (prev_p);
       result = dictionary_at (result, key);
       if (IS_NIL (result))
- return NULL;
+ {
+  free (s);
+  return NULL;
+ }
     }
 
   free (s);
diff --git a/libgst/cint.c b/libgst/cint.c
index 6848234a..a9585e82 100644
--- a/libgst/cint.c
+++ b/libgst/cint.c
@@ -837,7 +837,10 @@ _gst_invoke_croutine (OOP cFuncOOP,
 
   funcAddr = cobject_value (cFuncOOP);
   if (!funcAddr)
-    return (NULL);
+    {
+      INC_RESTORE_POINTER (incPtr);
+      return (NULL);
+    }
 
   p_slot = pointer_map_insert (cif_cache, cFuncOOP);
   if (!*p_slot)
diff --git a/libgst/dict.c b/libgst/dict.c
index 9acb44cc..6e11fc68 100644
--- a/libgst/dict.c
+++ b/libgst/dict.c
@@ -231,8 +231,8 @@ static void create_class (const class_definition *ci);
    NUMMETACLASSSUBCLASSES in the instance variable "subclasses" of the
    metaclass.  */
 static void create_metaclass (OOP class_oop,
-      int numSubClasses,
-      int numMetaclassSubClasses);
+      int numMetaclassSubClasses,
+      int numSubClasses);
 
 /* Finish initializing the metaclass METACLASSOOP.  */
 static void init_metaclass (OOP metaclassOOP);
@@ -1030,8 +1030,8 @@ init_smalltalk_dictionary (void)
   for (i = 0; i < numFeatures; i++)
     featuresArray->data[i] = _gst_intern_string (feature_strings[i]);
 
-  sprintf (fullVersionString, "GNU Smalltalk version %s",
-   VERSION PACKAGE_GIT_REVISION);
+  snprintf (fullVersionString, sizeof (fullVersionString),
+   "GNU Smalltalk version %s", VERSION PACKAGE_GIT_REVISION);
 
   add_smalltalk ("Smalltalk", _gst_smalltalk_dictionary);
   add_smalltalk ("Version", _gst_string_new (fullVersionString));
diff --git a/libgst/oop.c b/libgst/oop.c
index 71e837a0..ed2b9492 100644
--- a/libgst/oop.c
+++ b/libgst/oop.c
@@ -787,7 +787,7 @@ _gst_alloc_obj (size_t size,
   if UNCOMMON (newAllocPtr >= _gst_mem.eden.maxPtr)
     {
       _gst_scavenge ();
-      newAllocPtr = _gst_mem.eden.allocPtr + size;
+      newAllocPtr = _gst_mem.eden.allocPtr + BYTES_TO_SIZE (size);
     }
 
   p_instance = (gst_object) _gst_mem.eden.allocPtr;
diff --git a/libgst/oop.h b/libgst/oop.h
index efc51da0..acc488db 100644
--- a/libgst/oop.h
+++ b/libgst/oop.h
@@ -75,7 +75,7 @@
 /* The number of OOPs in the system.  This is exclusive of Character,
    True, False, and UndefinedObject (nil) oops, which are
    built-ins.  */
-#define INITIAL_OOP_TABLE_SIZE (1024 * 128 + BUILTIN_OBJECT_BASE)
+#define INITIAL_OOP_TABLE_SIZE (1024 * 128 + FIRST_OOP_INDEX)
 #define MAX_OOP_TABLE_SIZE (1 << 23)
 
 /* The number of free OOPs under which we trigger GCs.  0 is not
@@ -413,7 +413,7 @@ extern gst_object _gst_alloc_words (size_t size)
 
 /* Grows the allocated memory to SPACESIZE bytes, if it's not there
    already. */
-extern void _gst_grow_memory_to (size_t size)
+extern void _gst_grow_memory_to (size_t spaceSize)
   ATTRIBUTE_HIDDEN;
 
 /* Grow the OOP table to NEWSIZE pointers and initialize the newly
--
2.13.5


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

2017-04-26  Lee Duhem  <[hidden email]>

       * gstpriv.h (COUNT_OF): New macro.
       * dict.c (_gst_init_dictionary, _gst_init_dictionary_on_image_load):
       Use COUNT_OF.
       * sym.c (_gst_init_symbols_pass1): Ditto.
---
 libgst/ChangeLog | 7 +++++++
 libgst/dict.c    | 8 +++-----
 libgst/gstpriv.h | 5 +++++
 libgst/sym.c     | 6 +++---
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/libgst/ChangeLog b/libgst/ChangeLog
index 4d1375cc..5298b126 100644
--- a/libgst/ChangeLog
+++ b/libgst/ChangeLog
@@ -1,5 +1,12 @@
 2017-04-26  Lee Duhem  <[hidden email]>
 
+ * gstpriv.h (COUNT_OF): New macro.
+ * dict.c (_gst_init_dictionary, _gst_init_dictionary_on_image_load):
+ Use COUNT_OF.
+ * sym.c (_gst_init_symbols_pass1): Ditto.
+
+2017-04-26  Lee Duhem  <[hidden email]>
+
  * callin.c (_gst_type_name_to_oop): Replace sprintf with snprintf.
  (_gst_class_name_to_oop): Clean properly before return.
  * cint.c (_gst_invoke_croutine): Clean properly before return.
diff --git a/libgst/dict.c b/libgst/dict.c
index 6e11fc68..30845cef 100644
--- a/libgst/dict.c
+++ b/libgst/dict.c
@@ -828,13 +828,13 @@ _gst_init_dictionary (void)
 
   _gst_init_symbols_pass1 ();
 
-  create_classes_pass1 (class_info, sizeof (class_info) / sizeof (class_info[0]));
+  create_classes_pass1 (class_info, COUNT_OF (class_info));
 
   init_proto_oops();
   _gst_init_symbols_pass2 ();
   init_smalltalk_dictionary ();
 
-  create_classes_pass2 (class_info, sizeof (class_info) / sizeof (class_info[0]));
+  create_classes_pass2 (class_info, COUNT_OF (class_info));
 
   init_runtime_objects ();
   _gst_tenure_all_survivors ();
@@ -1308,9 +1308,7 @@ _gst_init_dictionary_on_image_load (mst_Boolean prim_table_matches)
 
   _gst_restore_symbols ();
 
-  for (ci = class_info;
-       ci < class_info + sizeof(class_info) / sizeof(class_definition);
-       ci++)
+  for (ci = class_info; ci < class_info + COUNT_OF (class_info); ci++)
     if (ci->reloadAddress)
       {
  *ci->classVar = dictionary_at (_gst_smalltalk_dictionary,
diff --git a/libgst/gstpriv.h b/libgst/gstpriv.h
index 85aa9a05..ed68ef22 100644
--- a/libgst/gstpriv.h
+++ b/libgst/gstpriv.h
@@ -553,6 +553,11 @@ extern OOP _gst_nil_oop
 #define MIN(x, y) ( ((x) > (y)) ? (y) : (x) )
 #endif
 
+#ifndef COUNT_OF
+#define COUNT_OF(x) ((sizeof(x)/sizeof(0[x])) / \
+ ((size_t)(!(sizeof(x) % sizeof(0[x])))))
+#endif
+
 #include "ansidecl.h"
 #include "mathl.h"
 #include "socketx.h"
diff --git a/libgst/sym.c b/libgst/sym.c
index 57f62e74..cbd0d325 100644
--- a/libgst/sym.c
+++ b/libgst/sym.c
@@ -1568,7 +1568,7 @@ _gst_init_symbols_pass1 (void)
      to the hash table entries.  */
   for (bs = _gst_builtin_selectors_hash;
        bs - _gst_builtin_selectors_hash <
- sizeof (_gst_builtin_selectors_hash) / sizeof (_gst_builtin_selectors_hash[0]);
+ COUNT_OF (_gst_builtin_selectors_hash);
        bs++)
     if (bs->offset != -1)
       {
@@ -1594,7 +1594,7 @@ _gst_init_symbols_pass2 (void)
      to the hash table entries.  */
   for (bs = _gst_builtin_selectors_hash;
        bs - _gst_builtin_selectors_hash <
- sizeof (_gst_builtin_selectors_hash) / sizeof (_gst_builtin_selectors_hash[0]);
+ COUNT_OF (_gst_builtin_selectors_hash);
        bs++)
     if (bs->offset != -1)
       {
@@ -1633,7 +1633,7 @@ _gst_restore_symbols (void)
      to the hash table entries.  */
   for (bs = _gst_builtin_selectors_hash;
        bs - _gst_builtin_selectors_hash <
- sizeof (_gst_builtin_selectors_hash) / sizeof (_gst_builtin_selectors_hash[0]);
+ COUNT_OF (_gst_builtin_selectors_hash);
        bs++)
     if (bs->offset != -1)
       {
--
2.13.5


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

Looks good.  I hope your changes get integrated.

Can you tell me what "This version 2 have no change since v1, only rebased
to top of master
branch." means?

Thanks!

Blake


On Thu, Nov 30, 2017 at 3:31 AM, Lee Duhem <[hidden email]> wrote:
_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

> On 30. Nov 2017, at 18:11, Blake McBride <[hidden email]> wrote:
>

Hey!


> Looks good.  I hope your changes get integrated.
>
> Can you tell me what "This version 2 have no change since v1, only rebased to top of master
> branch." means?

I had asked Lee to rebase the changes so I can more easily apply them. He is referring to the fact that there was no functional change between the first time he sent them and now.

cheers

        holger


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

On Thu, Nov 30, 2017 at 6:17 PM, Holger Freyther <[hidden email]> wrote:

Thanks for explaining that :-)

Regards,
lee


>
> cheers
>
>         holger
>
>
_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

> On 30. Nov 2017, at 17:31, Lee Duhem <[hidden email]> wrote:
>

Hi again!



> @@ -2025,7 +2023,7 @@ _gst_to_wide_cstring (OOP stringOOP)
>  string = (gst_unicode_string) OOP_TO_OBJ (stringOOP);
>  len = oop_num_fields (stringOOP);
>  result = (wchar_t *) xmalloc (len + 1);
> -  if (sizeof (wchar_t) == 4)
> +  if (sizeof (wchar_t) == sizeof(string->chars[0]))

Okay. string->chars[0] is uint32_t so the above is identical but
the code looks bad in general?

len = oop_num_fields (stringOOP);
result = (wchar_t *) xmalloc (len + 1);


'12345678' asUnicodeString basicSize
=> 8

There is barely any C API using "wchar_t" so it is no surprise
the code might be broken. I think it needs to be:

result = (wchar_t *) xmalloc ((len + 1) * sizeof(*result));

and then maybe size(*result) == sizeof(string->chars[0])



> -      && (flags & (GST_REBUILD_IMAGE | GST_MAYBE_REBUILD_IMAGE)) == 0
> +      && !rebuild_image_flags

I am flying right now but what is the GNU coding style? I have
my phases with !flag or flag == 0. I would only change it if there
is either a majority in our code or GNU coding style has a
statement about it.

cheers
        holger

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

> On 30. Nov 2017, at 17:31, Lee Duhem <[hidden email]> wrote:
>

Hey!



> /* Grows the allocated memory to SPACESIZE bytes, if it's not there
> -   already.  
> -   the memory could not be allocated.  Should be called after the
> -   sweep has occurred so that things are contiguous.  Ensures that the
> -   OOP table pointers are fixed up to point to the new objects.  */
> +   already. */
> extern void _gst_grow_memory_to (size_t size)

The "sweep" doesn't seem to be enforced (scavenge before as part of
the init_mem, but if invoked through the primitive I don't see a sweep).


Mentioning "_gst_fixup_object_pointers" and that no garbage is copied
might be valuable? I will most likely apply as is but why did you
decide to remove this?

cheers
        holger

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

Hello Holger,

On Sat, Dec 2, 2017 at 2:50 PM, Holger Freyther <[hidden email]> wrote:

The code implies that when sizeof(string->chars[0]) equals 4, it can memcpy
from
string->chars to result.  This change just wants to make that assumption
clear.


Yes, we need to xmalloc more spaces for the result.

BTW, adding an assertion to make sure sizeof(wchar_t) >=
sizeof(string->chars[0])
looks like a good idea to prevent possible overwritten.


I replaced that complicated condition with !rebuild_image_flags because we
already have that
variable, and it seems a good idea for me to simplify it.

About !flag or flag == 0, both work for me.  I chose !flag because there
are already several examples
in the same source code file.

Regards,
lee


>
> cheers
>         holger
>
_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk

Hello Holger,

On Sat, Dec 2, 2017 at 2:51 PM, Holger Freyther <[hidden email]> wrote:

We should mention that this function should be called after memory is
properly initialized,
i.e. at least keep statement "Should be called after ..."


>
>
> Mentioning "_gst_fixup_object_pointers" and that no garbage is copied
> might be valuable? I will most likely apply as is but why did you
> decide to remove this?
>

What about saying something like "Compact old objects and grow oldspace to
SPACESIZE bytes,
if it's not there already.  Should be called after the sweep has occurred."

I removed these statements because it seems that they are does not match
with current
implementation of _gst_grow_memory_to ().  However, I did not find all the
change history
of that function, so maybe I have removed too much.

Regards,
lee


>
> cheers
>         holger
>
_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk