From 95c3300ca645f60098218965b1afd45352917eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:42 +0200 Subject: [PATCH 1/8] Remove long_u: do_outofmem_msg(). Remove long_u occurrences due to do_outofmem_msg() function. Refactor size parameter from long_u into size_t. --- src/nvim/memory.c | 4 ++-- src/nvim/memory.h | 2 +- src/nvim/screen.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nvim/memory.c b/src/nvim/memory.c index 9bfb12e0ee..b6990890a6 100644 --- a/src/nvim/memory.c +++ b/src/nvim/memory.c @@ -84,7 +84,7 @@ void *verbose_try_malloc(size_t size) { void *ret = try_malloc(size); if (!ret) { - do_outofmem_msg((long_u)size); + do_outofmem_msg(size); } return ret; } @@ -214,7 +214,7 @@ void *xmemdup(const void *data, size_t len) * Avoid repeating the error message many times (they take 1 second each). * Did_outofmem_msg is reset when a character is read. */ -void do_outofmem_msg(long_u size) +void do_outofmem_msg(size_t size) { if (!did_outofmem_msg) { /* Don't hide this message */ diff --git a/src/nvim/memory.h b/src/nvim/memory.h index ac7b1ebd6a..2723a7ba80 100644 --- a/src/nvim/memory.h +++ b/src/nvim/memory.h @@ -134,7 +134,7 @@ char *xstpncpy(char *restrict dst, const char *restrict src, size_t maxlen); void *xmemdup(const void *data, size_t len) FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_RET; -void do_outofmem_msg(long_u size); +void do_outofmem_msg(size_t size); void free_all_mem(void); #endif diff --git a/src/nvim/screen.c b/src/nvim/screen.c index af3f5c999f..f4616d3b8e 100644 --- a/src/nvim/screen.c +++ b/src/nvim/screen.c @@ -6288,7 +6288,7 @@ retry: || outofmem) { if (ScreenLines != NULL || !done_outofmem_msg) { /* guess the size */ - do_outofmem_msg((long_u)((Rows + 1) * Columns)); + do_outofmem_msg((Rows + 1) * Columns); /* Remember we did this to avoid getting outofmem messages over * and over again. */ From 98255c7a78d5c9ccbbc580160d13dec2978a081a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:43 +0200 Subject: [PATCH 2/8] Remove long_u: hashtab: Cleanup: Comments. - Restyle comments (/// when appropiate, // otherwise). - Improve comments (add new comments, augment/clarify existing ones). --- src/nvim/hashtab.c | 134 ++++++++++++++++++++++----------------------- src/nvim/hashtab.h | 75 ++++++++++++++++--------- 2 files changed, 115 insertions(+), 94 deletions(-) diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c index 7445a4cec1..e8895cb68d 100644 --- a/src/nvim/hashtab.c +++ b/src/nvim/hashtab.c @@ -2,19 +2,19 @@ /// /// Handling of a hashtable with Vim-specific properties. /// -/// Each item in a hashtable has a NUL terminated string key. A key can appear +/// Each item in a hashtable has a NUL terminated string key. A key can appear /// only once in the table. /// -/// A hash number is computed from the key for quick lookup. When the hashes +/// A hash number is computed from the key for quick lookup. When the hashes /// of two different keys point to the same entry an algorithm is used to /// iterate over other entries in the table until the right one is found. /// To make the iteration work removed keys are different from entries where a /// key was never present. /// /// The mechanism has been partly based on how Python Dictionaries are -/// implemented. The algorithm is from Knuth Vol. 3, Sec. 6.4. +/// implemented. The algorithm is from Knuth Vol. 3, Sec. 6.4. /// -/// The hashtable grows to accommodate more entries when needed. At least 1/3 +/// The hashtable grows to accommodate more entries when needed. At least 1/3 /// of the entries is empty to keep the lookup efficient (at the cost of extra /// memory). @@ -31,10 +31,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems); - /// Initialize an empty hash table. -/// -/// @param ht void hash_init(hashtab_T *ht) { // This zeroes all "ht_" entries and all the "hi_key" in "ht_smallarray". @@ -43,10 +40,10 @@ void hash_init(hashtab_T *ht) ht->ht_mask = HT_INIT_SIZE - 1; } -/// Free the array of a hash table. Does not free the items it contains! -/// If "ht" is not freed then you should call hash_init() next! +/// Free the array of a hash table without freeing contained values. /// -/// @param ht +/// If "ht" is not freed (after calling this) then you should call hash_init() +/// right next! void hash_clear(hashtab_T *ht) { if (ht->ht_array != ht->ht_smallarray) { @@ -54,12 +51,9 @@ void hash_clear(hashtab_T *ht) } } -/// Free the array of a hash table and all the keys it contains. The keys must -/// have been allocated. "off" is the offset from the start of the allocate -/// memory to the location of the key (it's always positive). +/// Free the array of a hash table and all contained values. /// -/// @param ht -/// @param off +/// @param off the offset from start of value to start of key (@see hashitem_T). void hash_clear_all(hashtab_T *ht, int off) { long todo; @@ -76,17 +70,15 @@ void hash_clear_all(hashtab_T *ht, int off) hash_clear(ht); } -/// Find "key" in hashtable "ht". "key" must not be NULL. -/// Always returns a pointer to a hashitem. If the item was not found then -/// HASHITEM_EMPTY() is TRUE. The pointer is then the place where the key -/// would be added. -/// WARNING: The returned pointer becomes invalid when the hashtable is changed -/// (adding, setting or removing an item)! +/// Find item for given "key" in hashtable "ht". /// -/// @param ht -/// @param key +/// @param key The key of the looked-for item. Must not be NULL. /// -/// @return Pointer to the hashitem stored with the given key. +/// @return Pointer to the hash item corresponding to the given key. +/// If not found, then return pointer to the empty item that would be +/// used for that key. +/// WARNING: Returned pointer becomes invalid as soon as the hash table +/// is changed in any way. hashitem_T* hash_find(hashtab_T *ht, char_u *key) { return hash_lookup(ht, key, hash_hash(key)); @@ -94,11 +86,14 @@ hashitem_T* hash_find(hashtab_T *ht, char_u *key) /// Like hash_find(), but caller computes "hash". /// -/// @param ht -/// @param key -/// @param hash +/// @param key The key of the looked-for item. Must not be NULL. +/// @param hash The precomputed hash for the key. /// -/// @return Pointer to the hashitem stored with the given key. +/// @return Pointer to the hashitem corresponding to the given key. +/// If not found, then return pointer to the empty item that would be +/// used for that key. +/// WARNING: Returned pointer becomes invalid as soon as the hash table +/// is changed in any way. hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) { hash_T perturb; @@ -129,9 +124,9 @@ hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) freeitem = NULL; } - // Need to search through the table to find the key. The algorithm + // Need to search through the table to find the key. The algorithm // to step through the table starts with large steps, gradually becoming - // smaller down to (1/4 table size + 1). This means it goes through all + // smaller down to (1/4 table size + 1). This means it goes through all // table entries in the end. // When we run into a NULL key it's clear that the key isn't there. // Return the first available slot found (can be a slot of a removed @@ -161,6 +156,7 @@ hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) } /// Print the efficiency of hashtable lookups. +/// /// Useful when trying different hash algorithms. /// Called when exiting. void hash_debug_results(void) @@ -176,12 +172,13 @@ void hash_debug_results(void) #endif // ifdef HT_DEBUG } -/// Add item with key "key" to hashtable "ht". +/// Add item for key "key" to hashtable "ht". /// -/// @param ht -/// @param key +/// @param key Pointer to the key for the new item. The key has to be contained +/// in the new item (@see hashitem_T). Must not be NULL. /// -/// @returns FAIL when out of memory or the key is already present. +/// @return OK if success. +/// FAIL if key already present, or out of memory. int hash_add(hashtab_T *ht, char_u *key) { hash_T hash = hash_hash(key); @@ -193,16 +190,16 @@ int hash_add(hashtab_T *ht, char_u *key) return hash_add_item(ht, hi, key, hash); } -/// Add item "hi" with "key" to hashtable "ht". "key" must not be NULL and -/// "hi" must have been obtained with hash_lookup() and point to an empty item. -/// "hi" is invalid after this! +/// Add item "hi" for key "key" to hashtable "ht". /// -/// @param ht -/// @param hi -/// @param key -/// @param hash +/// @param hi The hash item to be used. Must have been obtained through +/// hash_lookup() and point to an empty item. +/// @param key Pointer to the key for the new item. The key has to be contained +/// in the new item (@see hashitem_T). Must not be NULL. +/// @param hash The precomputed hash value for the key. /// -/// @returns OK or FAIL (out of memory). +/// @return OK if success. +/// FAIL if out of memory. int hash_add_item(hashtab_T *ht, hashitem_T *hi, char_u *key, hash_T hash) { // If resizing failed before and it fails again we can't add an item. @@ -221,13 +218,12 @@ int hash_add_item(hashtab_T *ht, hashitem_T *hi, char_u *key, hash_T hash) return hash_may_resize(ht, 0); } -/// Remove item "hi" from hashtable "ht". "hi" must have been obtained with -/// hash_lookup(). +/// Remove item "hi" from hashtable "ht". +/// +/// Caller must take care of freeing the item itself. /// -/// The caller must take care of freeing the item itself. -/// -/// @param ht -/// @param hi +/// @param hi The hash item to be removed. +/// It must have been obtained with hash_lookup(). void hash_remove(hashtab_T *ht, hashitem_T *hi) { ht->ht_used--; @@ -235,18 +231,18 @@ void hash_remove(hashtab_T *ht, hashitem_T *hi) hash_may_resize(ht, 0); } -/// Lock a hashtable: prevent that ht_array changes. +/// Lock hashtable (prevent changes in ht_array). +/// /// Don't use this when items are to be added! /// Must call hash_unlock() later. -/// -/// @param ht void hash_lock(hashtab_T *ht) { ht->ht_locked++; } -/// Unlock a hashtable: allow ht_array changes again. -/// Table will be resized (shrink) when necessary. +/// Unlock hashtable (allow changes in ht_array again). +/// +/// Table will be resized (shrunk) when necessary. /// This must balance a call to hash_lock(). void hash_unlock(hashtab_T *ht) { @@ -254,13 +250,16 @@ void hash_unlock(hashtab_T *ht) (void)hash_may_resize(ht, 0); } -/// Shrink a hashtable when there is too much empty space. -/// Grow a hashtable when there is not enough empty space. +/// Resize hastable (new size can be given or automatically computed). /// -/// @param ht -/// @param minitems minimal number of items +/// @param minitems Minimum number of items the new table should hold. +/// If zero, new size will depend on currently used items: +/// - Shrink when too much empty space. +/// - Grow when not enough empty space. +/// If non-zero, passed minitems will be used. /// -/// @returns OK or FAIL (out of memory). +/// @return OK if success. +/// FAIL if out of memory. static int hash_may_resize(hashtab_T *ht, int minitems) { hashitem_T temparray[HT_INIT_SIZE]; @@ -289,7 +288,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) #endif // ifdef HT_DEBUG if (minitems == 0) { - // Return quickly for small tables with at least two NULL items. NULL + // Return quickly for small tables with at least two NULL items. // items are required for the lookup to decide a key isn't there. if ((ht->ht_filled < HT_INIT_SIZE - 1) && (ht->ht_array == ht->ht_smallarray)) { @@ -298,7 +297,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) // Grow or refill the array when it's more than 2/3 full (including // removed items, so that they get cleaned up). - // Shrink the array when it's less than 1/5 full. When growing it is + // Shrink the array when it's less than 1/5 full. When growing it is // at least 1/4 full (avoids repeated grow-shrink operations) oldsize = ht->ht_mask + 1; if ((ht->ht_filled * 3 < oldsize * 2) && (ht->ht_used > oldsize / 5)) { @@ -338,7 +337,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) newarray = ht->ht_smallarray; if (ht->ht_array == newarray) { // Moving from ht_smallarray to ht_smallarray! Happens when there - // are many removed items. Copy the items to be able to clean up + // are many removed items. Copy the items to be able to clean up // removed items. memmove(temparray, newarray, sizeof(temparray)); oldarray = temparray; @@ -353,7 +352,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) memset(newarray, 0, (size_t)(sizeof(hashitem_T) * newsize)); // Move all the items from the old array to the new one, placing them in - // the right spot. The new array won't have any removed items, thus this + // the right spot. The new array won't have any removed items, thus this // is also a cleanup action. newmask = newsize - 1; todo = (int)ht->ht_used; @@ -361,7 +360,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) for (olditem = oldarray; todo > 0; ++olditem) { if (!HASHITEM_EMPTY(olditem)) { // The algorithm to find the spot to add the item is identical to - // the algorithm to find an item in hash_lookup(). But we only + // the algorithm to find an item in hash_lookup(). But we only // need to search for a NULL key, thus it's simpler. newi = (unsigned)(olditem->hi_hash & newmask); newitem = &newarray[newi]; @@ -391,14 +390,11 @@ static int hash_may_resize(hashtab_T *ht, int minitems) } /// Get the hash number for a key. +/// /// If you think you know a better hash function: Compile with HT_DEBUG set and -/// run a script that uses hashtables a lot. Vim will then print statistics -/// when exiting. Try that with the current hash algorithm and yours. The +/// run a script that uses hashtables a lot. Vim will then print statistics +/// when exiting. Try that with the current hash algorithm and yours. The /// lower the percentage the better. -/// -/// @param key -/// -/// @return Hash number for the key. hash_T hash_hash(char_u *key) { hash_T hash; diff --git a/src/nvim/hashtab.h b/src/nvim/hashtab.h index 118795efe6..8abb94b9a1 100644 --- a/src/nvim/hashtab.h +++ b/src/nvim/hashtab.h @@ -1,44 +1,69 @@ #ifndef NVIM_HASHTAB_H #define NVIM_HASHTAB_H -/* Item for a hashtable. "hi_key" can be one of three values: - * NULL: Never been used - * HI_KEY_REMOVED: Entry was removed - * Otherwise: Used item, pointer to the actual key; this usually is - * inside the item, subtract an offset to locate the item. - * This reduces the size of hashitem by 1/3. - */ +/// A hastable item. +/// +/// Each item has a NUL terminated string key. +/// A key can appear only once in the table. +/// +/// A hash number is computed from the key for quick lookup. When the hashes +/// of two different keys point to the same entry an algorithm is used to +/// iterate over other entries in the table until the right one is found. +/// To make the iteration work removed keys are different from entries where a +/// key was never present. +/// +/// Note that this does not contain a pointer to the key and another pointer to +/// the value. Instead, it is assumed that the key is contained within the +/// value, so that you can get a pointer to the value subtracting an offset from +/// the pointer to the key. +/// This reduces the size of this item by 1/3. typedef struct hashitem_S { - long_u hi_hash; /* cached hash number of hi_key */ - char_u *hi_key; + /// Cached hash number for hi_key. + long_u hi_hash; + + /// Item key. + /// + /// Possible values mean the following: + /// NULL : Item was never used. + /// HI_KEY_REMOVED : Item was removed. + /// (Any other pointer value) : Item is currently being used. + char_u *hi_key; } hashitem_T; -/* The address of "hash_removed" is used as a magic number for hi_key to - * indicate a removed item. */ +/// The address of "hash_removed" is used as a magic number +/// for hi_key to indicate a removed item. #define HI_KEY_REMOVED &hash_removed #define HASHITEM_EMPTY(hi) ((hi)->hi_key == NULL || (hi)->hi_key == \ &hash_removed) -/* Initial size for a hashtable. Our items are relatively small and growing - * is expensive, thus use 16 as a start. Must be a power of 2. */ +/// Initial size for a hashtable. +/// Our items are relatively small and growing is expensive, thus start with 16. +/// Must be a power of 2. #define HT_INIT_SIZE 16 +/// An array-based hashtable. +/// +/// Keys are NUL terminated strings. They cannot be repeated within a table. +/// Values are of any type. +/// +/// The hashtable grows to accommodate more entries when needed. typedef struct hashtable_S { - long_u ht_mask; /* mask used for hash value (nr of items in - * array is "ht_mask" + 1) */ - long_u ht_used; /* number of items used */ - long_u ht_filled; /* number of items used + removed */ - int ht_locked; /* counter for hash_lock() */ - int ht_error; /* when set growing failed, can't add more - items before growing works */ - hashitem_T *ht_array; /* points to the array, allocated when it's - not "ht_smallarray" */ - hashitem_T ht_smallarray[HT_INIT_SIZE]; /* initial array */ + long_u ht_mask; /// mask used for hash value + /// (nr of items in array is "ht_mask" + 1) + long_u ht_used; /// number of items used + long_u ht_filled; /// number of items used or removed + int ht_locked; /// counter for hash_lock() + int ht_error; /// when set growing failed, can't add more + /// items before growing works + hashitem_T *ht_array; /// points to the array, allocated when it's + /// not "ht_smallarray" + hashitem_T ht_smallarray[HT_INIT_SIZE]; /// initial array } hashtab_T; -typedef long_u hash_T; /* Type for hi_hash */ +/// Type for hash number (hash calculation result). +typedef long_u hash_T; -/* hashtab.c */ +// hashtab.c void hash_init(hashtab_T *ht); void hash_clear(hashtab_T *ht); void hash_clear_all(hashtab_T *ht, int off); From 4d97ae66f941d9817a8b4857a34fd7ba237a2861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:45 +0200 Subject: [PATCH 3/8] Remove long_u: hashtab: Cleanup: Others. hashtab.h: - Add missing includes. - Move hash_T to the top and use it to define hashtab_T. - Move hash_removed related definitions to the top, as they are used in the definition of hashtab_T. - Reformat multiline expression (start continuation with operator). - Reformat function declaration into one single line. hashtab.c: - Use C99 style variable declarations (move declarations as near to first-usage point as possible). - Simplify oldarray/newarray computation. - Simplify unneeded else branch. - Remove redundant casts. --- src/nvim/hashtab.c | 91 +++++++++++++++++----------------------------- src/nvim/hashtab.h | 25 +++++++------ 2 files changed, 47 insertions(+), 69 deletions(-) diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c index e8895cb68d..fe3a3e6dc5 100644 --- a/src/nvim/hashtab.c +++ b/src/nvim/hashtab.c @@ -18,6 +18,7 @@ /// of the entries is empty to keep the lookup efficient (at the cost of extra /// memory). +#include #include #include "nvim/vim.h" @@ -56,12 +57,8 @@ void hash_clear(hashtab_T *ht) /// @param off the offset from start of value to start of key (@see hashitem_T). void hash_clear_all(hashtab_T *ht, int off) { - long todo; - hashitem_T *hi; - - todo = (long)ht->ht_used; - - for (hi = ht->ht_array; todo > 0; ++hi) { + long todo = (long)ht->ht_used; + for (hashitem_T *hi = ht->ht_array; todo > 0; ++hi) { if (!HASHITEM_EMPTY(hi)) { free(hi->hi_key - off); todo--; @@ -96,11 +93,6 @@ hashitem_T* hash_find(hashtab_T *ht, char_u *key) /// is changed in any way. hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) { - hash_T perturb; - hashitem_T *freeitem; - hashitem_T *hi; - unsigned idx; - #ifdef HT_DEBUG hash_count_lookup++; #endif // ifdef HT_DEBUG @@ -109,19 +101,18 @@ hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) // - return if there is no item at all // - skip over a removed item // - return if the item matches - idx = (unsigned)(hash & ht->ht_mask); - hi = &ht->ht_array[idx]; + unsigned idx = (unsigned)(hash & ht->ht_mask); + hashitem_T *hi = &ht->ht_array[idx]; if (hi->hi_key == NULL) { return hi; } + hashitem_T *freeitem = NULL; if (hi->hi_key == HI_KEY_REMOVED) { freeitem = hi; } else if ((hi->hi_hash == hash) && (STRCMP(hi->hi_key, key) == 0)) { return hi; - } else { - freeitem = NULL; } // Need to search through the table to find the key. The algorithm @@ -131,7 +122,7 @@ hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) // When we run into a NULL key it's clear that the key isn't there. // Return the first available slot found (can be a slot of a removed // item). - for (perturb = hash;; perturb >>= PERTURB_SHIFT) { + for (hash_T perturb = hash;; perturb >>= PERTURB_SHIFT) { #ifdef HT_DEBUG // count a "miss" for hashtab lookup hash_count_perturb++; @@ -247,7 +238,7 @@ void hash_lock(hashtab_T *ht) void hash_unlock(hashtab_T *ht) { ht->ht_locked--; - (void)hash_may_resize(ht, 0); + hash_may_resize(ht, 0); } /// Resize hastable (new size can be given or automatically computed). @@ -262,16 +253,6 @@ void hash_unlock(hashtab_T *ht) /// FAIL if out of memory. static int hash_may_resize(hashtab_T *ht, int minitems) { - hashitem_T temparray[HT_INIT_SIZE]; - hashitem_T *oldarray, *newarray; - hashitem_T *olditem, *newitem; - unsigned newi; - int todo; - long_u oldsize, newsize; - long_u minsize; - long_u newmask; - hash_T perturb; - // Don't resize a locked table. if (ht->ht_locked > 0) { return OK; @@ -287,6 +268,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) } #endif // ifdef HT_DEBUG + long_u minsize; if (minitems == 0) { // Return quickly for small tables with at least two NULL items. // items are required for the lookup to decide a key isn't there. @@ -299,7 +281,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) // removed items, so that they get cleaned up). // Shrink the array when it's less than 1/5 full. When growing it is // at least 1/4 full (avoids repeated grow-shrink operations) - oldsize = ht->ht_mask + 1; + long_u oldsize = ht->ht_mask + 1; if ((ht->ht_filled * 3 < oldsize * 2) && (ht->ht_used > oldsize / 5)) { return OK; } @@ -321,8 +303,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) minsize = minitems * 3 / 2; } - newsize = HT_INIT_SIZE; - + long_u newsize = HT_INIT_SIZE; while (newsize < minsize) { // make sure it's always a power of 2 newsize <<= 1; @@ -332,40 +313,37 @@ static int hash_may_resize(hashtab_T *ht, int minitems) } } - if (newsize == HT_INIT_SIZE) { - // Use the small array inside the hashdict structure. - newarray = ht->ht_smallarray; - if (ht->ht_array == newarray) { - // Moving from ht_smallarray to ht_smallarray! Happens when there - // are many removed items. Copy the items to be able to clean up - // removed items. - memmove(temparray, newarray, sizeof(temparray)); - oldarray = temparray; - } else { - oldarray = ht->ht_array; - } - } else { - // Allocate an array. - newarray = xmalloc(sizeof(hashitem_T) * newsize); - oldarray = ht->ht_array; - } + bool newarray_is_small = newsize == HT_INIT_SIZE; + bool keep_smallarray = newarray_is_small + && ht->ht_array == ht->ht_smallarray; + + // Make sure that oldarray and newarray do not overlap, + // so that copying is possible. + hashitem_T temparray[HT_INIT_SIZE]; + hashitem_T *oldarray = keep_smallarray + ? memcpy(temparray, ht->ht_smallarray, sizeof(temparray)) + : ht->ht_array; + hashitem_T *newarray = newarray_is_small + ? ht->ht_smallarray + : xmalloc(sizeof(hashitem_T) * newsize); + memset(newarray, 0, (size_t)(sizeof(hashitem_T) * newsize)); // Move all the items from the old array to the new one, placing them in // the right spot. The new array won't have any removed items, thus this // is also a cleanup action. - newmask = newsize - 1; - todo = (int)ht->ht_used; + long_u newmask = newsize - 1; + int todo = (int)ht->ht_used; - for (olditem = oldarray; todo > 0; ++olditem) { + for (hashitem_T *olditem = oldarray; todo > 0; ++olditem) { if (!HASHITEM_EMPTY(olditem)) { // The algorithm to find the spot to add the item is identical to // the algorithm to find an item in hash_lookup(). But we only // need to search for a NULL key, thus it's simpler. - newi = (unsigned)(olditem->hi_hash & newmask); - newitem = &newarray[newi]; + unsigned newi = (unsigned)(olditem->hi_hash & newmask); + hashitem_T *newitem = &newarray[newi]; if (newitem->hi_key != NULL) { - for (perturb = olditem->hi_hash;; perturb >>= PERTURB_SHIFT) { + for (hash_T perturb = olditem->hi_hash;; perturb >>= PERTURB_SHIFT) { newi = 5 * newi + perturb + 1; newitem = &newarray[newi & newmask]; if (newitem->hi_key == NULL) { @@ -397,17 +375,16 @@ static int hash_may_resize(hashtab_T *ht, int minitems) /// lower the percentage the better. hash_T hash_hash(char_u *key) { - hash_T hash; - char_u *p; + hash_T hash = *key; - if ((hash = *key) == 0) { + if (hash == 0) { // Empty keys are not allowed, but we don't want to crash if we get one. return (hash_T) 0; } - p = key + 1; // A simplistic algorithm that appears to do very well. // Suggested by George Reilly. + char_u *p = key + 1; while (*p != NUL) { hash = hash * 101 + *p++; } diff --git a/src/nvim/hashtab.h b/src/nvim/hashtab.h index 8abb94b9a1..b556c6a9f3 100644 --- a/src/nvim/hashtab.h +++ b/src/nvim/hashtab.h @@ -1,6 +1,17 @@ #ifndef NVIM_HASHTAB_H #define NVIM_HASHTAB_H +#include "nvim/vim.h" + +/// Type for hash number (hash calculation result). +typedef long_u hash_T; + +/// The address of "hash_removed" is used as a magic number +/// for hi_key to indicate a removed item. +#define HI_KEY_REMOVED &hash_removed +#define HASHITEM_EMPTY(hi) ((hi)->hi_key == NULL \ + || (hi)->hi_key == &hash_removed) + /// A hastable item. /// /// Each item has a NUL terminated string key. @@ -19,7 +30,7 @@ /// This reduces the size of this item by 1/3. typedef struct hashitem_S { /// Cached hash number for hi_key. - long_u hi_hash; + hash_T hi_hash; /// Item key. /// @@ -30,12 +41,6 @@ typedef struct hashitem_S { char_u *hi_key; } hashitem_T; -/// The address of "hash_removed" is used as a magic number -/// for hi_key to indicate a removed item. -#define HI_KEY_REMOVED &hash_removed -#define HASHITEM_EMPTY(hi) ((hi)->hi_key == NULL || (hi)->hi_key == \ - &hash_removed) - /// Initial size for a hashtable. /// Our items are relatively small and growing is expensive, thus start with 16. /// Must be a power of 2. @@ -60,9 +65,6 @@ typedef struct hashtable_S { hashitem_T ht_smallarray[HT_INIT_SIZE]; /// initial array } hashtab_T; -/// Type for hash number (hash calculation result). -typedef long_u hash_T; - // hashtab.c void hash_init(hashtab_T *ht); void hash_clear(hashtab_T *ht); @@ -71,8 +73,7 @@ hashitem_T *hash_find(hashtab_T *ht, char_u *key); hashitem_T *hash_lookup(hashtab_T *ht, char_u *key, hash_T hash); void hash_debug_results(void); int hash_add(hashtab_T *ht, char_u *key); -int hash_add_item(hashtab_T *ht, hashitem_T *hi, char_u *key, - hash_T hash); +int hash_add_item(hashtab_T *ht, hashitem_T *hi, char_u *key, hash_T hash); void hash_remove(hashtab_T *ht, hashitem_T *hi); void hash_lock(hashtab_T *ht); void hash_unlock(hashtab_T *ht); From 0c68623aca08fa84c7ed945bac7d5066de84d7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:46 +0200 Subject: [PATCH 4/8] Remove long_u: hashtab: Enable -Wconversion. - Add hashtab.c to converted files list. - Fix conversion issues: * hash_lookup() : idx : unsigned -> hash_T. * hash_may_resize() : minitems : int -> size_t. * hash_may_resize() : newi : unsigned -> hash_T. * hash_may_resize() : minsize : long_u -> size_t. --- src/nvim/CMakeLists.txt | 1 + src/nvim/hashtab.c | 14 +++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt index ad45f3ad3b..eb270eecb5 100644 --- a/src/nvim/CMakeLists.txt +++ b/src/nvim/CMakeLists.txt @@ -37,6 +37,7 @@ set(CONV_SRCS api.c arabic.c garray.c + hashtab.c memory.c map.c os/env.c diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c index fe3a3e6dc5..664c4eaf92 100644 --- a/src/nvim/hashtab.c +++ b/src/nvim/hashtab.c @@ -30,7 +30,7 @@ // Magic value for algorithm that walks through the array. #define PERTURB_SHIFT 5 -static int hash_may_resize(hashtab_T *ht, int minitems); +static int hash_may_resize(hashtab_T *ht, size_t minitems); /// Initialize an empty hash table. void hash_init(hashtab_T *ht) @@ -101,7 +101,7 @@ hashitem_T* hash_lookup(hashtab_T *ht, char_u *key, hash_T hash) // - return if there is no item at all // - skip over a removed item // - return if the item matches - unsigned idx = (unsigned)(hash & ht->ht_mask); + hash_T idx = hash & ht->ht_mask; hashitem_T *hi = &ht->ht_array[idx]; if (hi->hi_key == NULL) { @@ -251,7 +251,7 @@ void hash_unlock(hashtab_T *ht) /// /// @return OK if success. /// FAIL if out of memory. -static int hash_may_resize(hashtab_T *ht, int minitems) +static int hash_may_resize(hashtab_T *ht, size_t minitems) { // Don't resize a locked table. if (ht->ht_locked > 0) { @@ -268,7 +268,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) } #endif // ifdef HT_DEBUG - long_u minsize; + size_t minsize; if (minitems == 0) { // Return quickly for small tables with at least two NULL items. // items are required for the lookup to decide a key isn't there. @@ -295,9 +295,9 @@ static int hash_may_resize(hashtab_T *ht, int minitems) } } else { // Use specified size. - if ((long_u)minitems < ht->ht_used) { + if (minitems < ht->ht_used) { // just in case... - minitems = (int)ht->ht_used; + minitems = ht->ht_used; } // array is up to 2/3 full minsize = minitems * 3 / 2; @@ -340,7 +340,7 @@ static int hash_may_resize(hashtab_T *ht, int minitems) // The algorithm to find the spot to add the item is identical to // the algorithm to find an item in hash_lookup(). But we only // need to search for a NULL key, thus it's simpler. - unsigned newi = (unsigned)(olditem->hi_hash & newmask); + hash_T newi = olditem->hi_hash & newmask; hashitem_T *newitem = &newarray[newi]; if (newitem->hi_key != NULL) { for (hash_T perturb = olditem->hi_hash;; perturb >>= PERTURB_SHIFT) { From ec89761e8a05fb93ed6bbde1055fbc866f7bce7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:47 +0200 Subject: [PATCH 5/8] Remove long_u: hashtab: Refactor long_u type. hashtab.h: - hash_T: long_u -> size_t. In principle, a hash value could thought of as just an unsigned number without size semantics (uint32_t or uint64_t). But it is used as index at some places, and so, size_t is also eligible. Therea re some places where assignments occur between hash_T and size_t variables, in both directions. Therefore, if we define hash_T to be of a type having a different width than that of size_t, we will have an incorrect assignment somewhere that will require an assert/guard. So the most sensible option here seems to do hast_T to be size_t too. - hashtab_T.ht_mask: long_u -> hash_T. Masks are used to be combined with hash_T values, so they should be of the same type. hashtab.c: - hash_may_resize(): oldsize: long_u -> size_t. - hash_may_resize(): newsize: long_u -> size_t. - hash_may_resize(): newmask: long_u -> hash_T. --- src/nvim/hashtab.c | 8 ++++---- src/nvim/hashtab.h | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c index 664c4eaf92..3f5e4cf4a1 100644 --- a/src/nvim/hashtab.c +++ b/src/nvim/hashtab.c @@ -281,7 +281,7 @@ static int hash_may_resize(hashtab_T *ht, size_t minitems) // removed items, so that they get cleaned up). // Shrink the array when it's less than 1/5 full. When growing it is // at least 1/4 full (avoids repeated grow-shrink operations) - long_u oldsize = ht->ht_mask + 1; + size_t oldsize = ht->ht_mask + 1; if ((ht->ht_filled * 3 < oldsize * 2) && (ht->ht_used > oldsize / 5)) { return OK; } @@ -303,7 +303,7 @@ static int hash_may_resize(hashtab_T *ht, size_t minitems) minsize = minitems * 3 / 2; } - long_u newsize = HT_INIT_SIZE; + size_t newsize = HT_INIT_SIZE; while (newsize < minsize) { // make sure it's always a power of 2 newsize <<= 1; @@ -327,12 +327,12 @@ static int hash_may_resize(hashtab_T *ht, size_t minitems) ? ht->ht_smallarray : xmalloc(sizeof(hashitem_T) * newsize); - memset(newarray, 0, (size_t)(sizeof(hashitem_T) * newsize)); + memset(newarray, 0, sizeof(hashitem_T) * newsize); // Move all the items from the old array to the new one, placing them in // the right spot. The new array won't have any removed items, thus this // is also a cleanup action. - long_u newmask = newsize - 1; + hash_T newmask = newsize - 1; int todo = (int)ht->ht_used; for (hashitem_T *olditem = oldarray; todo > 0; ++olditem) { diff --git a/src/nvim/hashtab.h b/src/nvim/hashtab.h index b556c6a9f3..8fba1c1ef5 100644 --- a/src/nvim/hashtab.h +++ b/src/nvim/hashtab.h @@ -4,7 +4,7 @@ #include "nvim/vim.h" /// Type for hash number (hash calculation result). -typedef long_u hash_T; +typedef size_t hash_T; /// The address of "hash_removed" is used as a magic number /// for hi_key to indicate a removed item. @@ -53,10 +53,10 @@ typedef struct hashitem_S { /// /// The hashtable grows to accommodate more entries when needed. typedef struct hashtable_S { - long_u ht_mask; /// mask used for hash value + hash_T ht_mask; /// mask used for hash value /// (nr of items in array is "ht_mask" + 1) - long_u ht_used; /// number of items used - long_u ht_filled; /// number of items used or removed + size_t ht_used; /// number of items used + size_t ht_filled; /// number of items used or removed int ht_locked; /// counter for hash_lock() int ht_error; /// when set growing failed, can't add more /// items before growing works From 0509556b93e4f3854aeb4f4841b1eb8d0d94d0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:49 +0200 Subject: [PATCH 6/8] Remove long_u: hashtab: Refactor other types. Current type of some other parameters/variables can be improved: - hashtab_T : ht_error : int -> bool. - hash_clear_all() : off : int -> unsigned int. - hash_clear_all() : todo : long -> size_t. - hash_may_resize() : todo : int -> size_t. --- src/nvim/hashtab.c | 8 ++++---- src/nvim/hashtab.h | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c index 3f5e4cf4a1..f31d2f2cbd 100644 --- a/src/nvim/hashtab.c +++ b/src/nvim/hashtab.c @@ -55,9 +55,9 @@ void hash_clear(hashtab_T *ht) /// Free the array of a hash table and all contained values. /// /// @param off the offset from start of value to start of key (@see hashitem_T). -void hash_clear_all(hashtab_T *ht, int off) +void hash_clear_all(hashtab_T *ht, unsigned int off) { - long todo = (long)ht->ht_used; + size_t todo = ht->ht_used; for (hashitem_T *hi = ht->ht_array; todo > 0; ++hi) { if (!HASHITEM_EMPTY(hi)) { free(hi->hi_key - off); @@ -333,7 +333,7 @@ static int hash_may_resize(hashtab_T *ht, size_t minitems) // the right spot. The new array won't have any removed items, thus this // is also a cleanup action. hash_T newmask = newsize - 1; - int todo = (int)ht->ht_used; + size_t todo = ht->ht_used; for (hashitem_T *olditem = oldarray; todo > 0; ++olditem) { if (!HASHITEM_EMPTY(olditem)) { @@ -362,7 +362,7 @@ static int hash_may_resize(hashtab_T *ht, size_t minitems) ht->ht_array = newarray; ht->ht_mask = newmask; ht->ht_filled = ht->ht_used; - ht->ht_error = FALSE; + ht->ht_error = false; return OK; } diff --git a/src/nvim/hashtab.h b/src/nvim/hashtab.h index 8fba1c1ef5..dbb7fd9181 100644 --- a/src/nvim/hashtab.h +++ b/src/nvim/hashtab.h @@ -1,6 +1,7 @@ #ifndef NVIM_HASHTAB_H #define NVIM_HASHTAB_H +#include #include "nvim/vim.h" /// Type for hash number (hash calculation result). @@ -58,7 +59,7 @@ typedef struct hashtable_S { size_t ht_used; /// number of items used size_t ht_filled; /// number of items used or removed int ht_locked; /// counter for hash_lock() - int ht_error; /// when set growing failed, can't add more + bool ht_error; /// when set growing failed, can't add more /// items before growing works hashitem_T *ht_array; /// points to the array, allocated when it's /// not "ht_smallarray" @@ -68,7 +69,7 @@ typedef struct hashtable_S { // hashtab.c void hash_init(hashtab_T *ht); void hash_clear(hashtab_T *ht); -void hash_clear_all(hashtab_T *ht, int off); +void hash_clear_all(hashtab_T *ht, unsigned int off); hashitem_T *hash_find(hashtab_T *ht, char_u *key); hashitem_T *hash_lookup(hashtab_T *ht, char_u *key, hash_T hash); void hash_debug_results(void); From ffed9bfae92d2fed0fa7af41186b948de8d51256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:50 +0200 Subject: [PATCH 7/8] Remove long_u: hashtab: Enable clint: Sort clint file. --- clint-files.txt | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/clint-files.txt b/clint-files.txt index 9518e7cb84..b2d06524ea 100644 --- a/clint-files.txt +++ b/clint-files.txt @@ -1,3 +1,14 @@ +src/nvim/api/buffer.c +src/nvim/api/buffer.h +src/nvim/api/defs.h +src/nvim/api/helpers.c +src/nvim/api/helpers.h +src/nvim/api/tabpage.c +src/nvim/api/tabpage.h +src/nvim/api/vim.c +src/nvim/api/vim.h +src/nvim/api/window.c +src/nvim/api/window.h src/nvim/indent.c src/nvim/indent.h src/nvim/log.c @@ -7,34 +18,23 @@ src/nvim/map.h src/nvim/map_defs.h src/nvim/os/env.c src/nvim/os/event.c -src/nvim/os/event_defs.h src/nvim/os/event.h +src/nvim/os/event_defs.h src/nvim/os/input.c src/nvim/os/input.h -src/nvim/os/rstream.c -src/nvim/os/rstream_defs.h -src/nvim/os/rstream.h src/nvim/os/job.c -src/nvim/os/job_defs.h src/nvim/os/job.h +src/nvim/os/job_defs.h src/nvim/os/mem.c +src/nvim/os/msgpack_rpc.c +src/nvim/os/msgpack_rpc.h src/nvim/os/os.h +src/nvim/os/rstream.c +src/nvim/os/rstream.h +src/nvim/os/rstream_defs.h src/nvim/os/shell.c src/nvim/os/shell.h src/nvim/os/signal.c src/nvim/os/signal.h src/nvim/os/time.c src/nvim/os/time.h -src/nvim/os/msgpack_rpc.h -src/nvim/os/msgpack_rpc.c -src/nvim/api/defs.h -src/nvim/api/buffer.h -src/nvim/api/buffer.c -src/nvim/api/helpers.h -src/nvim/api/helpers.c -src/nvim/api/tabpage.h -src/nvim/api/tabpage.c -src/nvim/api/window.h -src/nvim/api/window.c -src/nvim/api/vim.h -src/nvim/api/vim.c From f179081fc84c4f8b2bb80d731ee3936fbc075a32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eliseo=20Marti=CC=81nez?= Date: Sat, 24 May 2014 01:17:51 +0200 Subject: [PATCH 8/8] Remove long_u: hashtab: Enable clint: Add to clint. - Add hashtab.[ch] to clint file. - Fix clint errors. --- clint-files.txt | 2 ++ src/nvim/hashtab.c | 6 +++--- src/nvim/hashtab.h | 10 +++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/clint-files.txt b/clint-files.txt index b2d06524ea..c26ca9b964 100644 --- a/clint-files.txt +++ b/clint-files.txt @@ -9,6 +9,8 @@ src/nvim/api/vim.c src/nvim/api/vim.h src/nvim/api/window.c src/nvim/api/window.h +src/nvim/hashtab.c +src/nvim/hashtab.h src/nvim/indent.c src/nvim/indent.h src/nvim/log.c diff --git a/src/nvim/hashtab.c b/src/nvim/hashtab.c index f31d2f2cbd..4e4274bfa1 100644 --- a/src/nvim/hashtab.c +++ b/src/nvim/hashtab.c @@ -67,7 +67,7 @@ void hash_clear_all(hashtab_T *ht, unsigned int off) hash_clear(ht); } -/// Find item for given "key" in hashtable "ht". +/// Find item for given "key" in hashtable "ht". /// /// @param key The key of the looked-for item. Must not be NULL. /// @@ -210,7 +210,7 @@ int hash_add_item(hashtab_T *ht, hashitem_T *hi, char_u *key, hash_T hash) } /// Remove item "hi" from hashtable "ht". -/// +/// /// Caller must take care of freeing the item itself. /// /// @param hi The hash item to be removed. @@ -222,7 +222,7 @@ void hash_remove(hashtab_T *ht, hashitem_T *hi) hash_may_resize(ht, 0); } -/// Lock hashtable (prevent changes in ht_array). +/// Lock hashtable (prevent changes in ht_array). /// /// Don't use this when items are to be added! /// Must call hash_unlock() later. diff --git a/src/nvim/hashtab.h b/src/nvim/hashtab.h index dbb7fd9181..bd64984ac8 100644 --- a/src/nvim/hashtab.h +++ b/src/nvim/hashtab.h @@ -5,10 +5,10 @@ #include "nvim/vim.h" /// Type for hash number (hash calculation result). -typedef size_t hash_T; +typedef size_t hash_T; /// The address of "hash_removed" is used as a magic number -/// for hi_key to indicate a removed item. +/// for hi_key to indicate a removed item. #define HI_KEY_REMOVED &hash_removed #define HASHITEM_EMPTY(hi) ((hi)->hi_key == NULL \ || (hi)->hi_key == &hash_removed) @@ -34,7 +34,7 @@ typedef struct hashitem_S { hash_T hi_hash; /// Item key. - /// + /// /// Possible values mean the following: /// NULL : Item was never used. /// HI_KEY_REMOVED : Item was removed. @@ -44,7 +44,7 @@ typedef struct hashitem_S { /// Initial size for a hashtable. /// Our items are relatively small and growing is expensive, thus start with 16. -/// Must be a power of 2. +/// Must be a power of 2. #define HT_INIT_SIZE 16 /// An array-based hashtable. @@ -80,4 +80,4 @@ void hash_lock(hashtab_T *ht); void hash_unlock(hashtab_T *ht); hash_T hash_hash(char_u *key); -#endif /* NVIM_HASHTAB_H */ +#endif // NVIM_HASHTAB_H