On 23/03/15 22:10, Jan Vesely wrote:

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

...

Remove the hack of including C files, by reworking the only requirement drmOpenMinor() to an open(buf...). After all we do know the exact name of the device we're going to open, so might as well use it. Replace hard-coded 16 with DRM_MAX_MINOR while we're here.

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/dristat.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/dristat.c b/tests/dristat.c index cca4b03..cc23e16 100644 --- a/tests/dristat.c +++ b/tests/dristat.c @@ -31,13 +31,14 @@ # include <config.h> #endif

+#include <ctype.h> +#include <fcntl.h> #include <stdio.h> #include <stdlib.h> +#include <string.h> +#include <sys/stat.h> #include <unistd.h> #include "xf86drm.h" -#include "xf86drmRandom.c" -#include "xf86drmHash.c" -#include "xf86drm.c"

#define DRM_VERSION 0x00000001 #define DRM_MEMORY 0x00000002 @@ -267,9 +268,9 @@ int main(int argc, char **argv) return 1; }

• for (i = 0; i < 16; i++) if (!minor || i == minor) {

• for (i = 0; i < DRM_MAX_MINOR; i++) if (!minor || i == minor) { sprintf(buf, DRM_DEV_NAME, DRM_DIR_NAME, i);

• fd = drmOpenMinor(i, 1, DRM_NODE_PRIMARY);

• fd = open(buf, O_RDWR, 0);

What about the "create" (second) argument? The original code creates the node if it does not exist (on non-UDEV systems), or waits some time for udev to create it. Not sure how this is relevant for the test.

Hmm brain triggered this as "create = 0", as I was going through. Fwiw I would opt for nuking this custom hack, and stick with open().

If one needs to wait for udev or manually create the nod then they have bigger ship to fry. All of that should be resolved before anyone has the change to run the program.

Let's give it some time for people to voice their opinions on the topic. Could be that something more elaborate is happing if create is set.

-Emil

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

This way with follow up commits we can fix it and wire it up to make check

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/Makefile.am | 3 +- tests/hash.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ xf86drmHash.c | 174 +++---------------------------------------- 3 files changed, 231 insertions(+), 165 deletions(-) create mode 100644 tests/hash.c

diff --git a/tests/Makefile.am b/tests/Makefile.am index 10f54e3..ea826b5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,7 +29,8 @@ LDADD = $(top_builddir)/libdrm.la

check_PROGRAMS = \ dristat \

• drmstat

• drmstat \
• hash

if HAVE_NOUVEAU SUBDIRS += nouveau diff --git a/tests/hash.c b/tests/hash.c new file mode 100644 index 0000000..d57d2dc --- /dev/null +++ b/tests/hash.c @@ -0,0 +1,219 @@ +/* xf86drmHash.c -- Small hash table support for integer -> integer mapping

• • Created: Sun Apr 18 09:35:45 1999 by faith@precisioninsight.com
• • Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
• • All Rights Reserved.
• • Permission is hereby granted, free of charge, to any person obtaining a
• • copy of this software and associated documentation files (the "Software"),
• • to deal in the Software without restriction, including without limitation
• • the rights to use, copy, modify, merge, publish, distribute, sublicense,
• • and/or sell copies of the Software, and to permit persons to whom the
• • Software is furnished to do so, subject to the following conditions:
• • The above copyright notice and this permission notice (including the next
• • paragraph) shall be included in all copies or substantial portions of the
• • Software.
• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
• • IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
• • FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
• • PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
• • OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
• • ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
• • DEALINGS IN THE SOFTWARE.
• • Authors: Rickard E. (Rik) Faith faith@valinux.com
• • DESCRIPTION
• • This file contains a straightforward implementation of a fixed-sized
• • hash table using self-organizing linked lists [Knuth73, pp. 398-399] for
• • collision resolution. There are two potentially interesting things
• • about this implementation:
• • 1. The table is power-of-two sized. Prime sized tables are more
• • traditional, but do not have a significant advantage over power-of-two
• • sized table, especially when double hashing is not used for collision
• • resolution.
• • 2. The hash computation uses a table of random integers [Hanson97,
• • pp. 39-41].
• • FUTURE ENHANCEMENTS
• • With a table size of 512, the current implementation is sufficient for a
• • few hundred keys. Since this is well above the expected size of the
• • tables for which this implementation was designed, the implementation of
• • dynamic hash tables was postponed until the need arises. A common (and
• • naive) approach to dynamic hash table implementation simply creates a
• • new hash table when necessary, rehashes all the data into the new table,
• • and destroys the old table. The approach in [Larson88] is superior in
• • two ways: 1) only a portion of the table is expanded when needed,
• • distributing the expansion cost over several insertions, and 2) portions
• • of the table can be locked, enabling a scalable thread-safe
• • implementation.
• • REFERENCES
• • [Hanson97] David R. Hanson. C Interfaces and Implementations:
• • Techniques for Creating Reusable Software. Reading, Massachusetts:
• • Addison-Wesley, 1997.
• • [Knuth73] Donald E. Knuth. The Art of Computer Programming. Volume 3:
• • Sorting and Searching. Reading, Massachusetts: Addison-Wesley, 1973.
• • [Larson88] Per-Ake Larson. "Dynamic Hash Tables". CACM 31(4), April
• • 1988, pp. 446-457.
• */

+#include <stdio.h> +#include <stdlib.h>

+#include "xf86drm.h"

+#define HASH_SIZE 512 /* Good for about 100 entries */

• 		/* If you change this value, you probably
  

•                               have to change the HashHash hashing
  

•                               function! */
  

+typedef struct HashBucket {

• unsigned long key;
• void *value;
• struct HashBucket *next;

+} HashBucket, *HashBucketPtr;

+typedef struct HashTable {

• unsigned long magic;
• unsigned long entries;
• unsigned long hits; /* At top of linked list */
• unsigned long partials; /* Not at top of linked list */
• unsigned long misses; /* Not in table */
• HashBucketPtr buckets[HASH_SIZE];
• int p0;
• HashBucketPtr p1;

+} HashTable, *HashTablePtr;

I don't think it's a good idea to duplicate these. maybe having an internal header would be a better way to go.

+#define DIST_LIMIT 10 +static int dist[DIST_LIMIT];

+static void clear_dist(void) {

• int i;
• for (i = 0; i < DIST_LIMIT; i++) dist[i] = 0;

+}

+static int count_entries(HashBucketPtr bucket) +{

• int count = 0;
• for (; bucket; bucket = bucket->next) ++count;
• return count;

+}

+static void update_dist(int count) +{

• if (count >= DIST_LIMIT) ++dist[DIST_LIMIT-1];
• else ++dist[count];

+}

+static void compute_dist(HashTablePtr table) +{

• int i;
• HashBucketPtr bucket;
• printf("Entries = %ld, hits = %ld, partials = %ld, misses = %ld\n",

•   table->entries, table->hits, table->partials, table->misses);
  

• clear_dist();
• for (i = 0; i < HASH_SIZE; i++) {
• bucket = table->buckets[i];
• update_dist(count_entries(bucket));
• }
• for (i = 0; i < DIST_LIMIT; i++) {
• if (i != DIST_LIMIT-1) printf("%5d %10d\n", i, dist[i]);
• else printf("other %10d\n", dist[i]);
• }

+}

+static void check_table(HashTablePtr table,

• 	unsigned long key, unsigned long value)
  

+{

• unsigned long retval = 0;
• int retcode = drmHashLookup(table, key, &retval);
• switch (retcode) {
• case -1:
• printf("Bad magic = 0x%08lx:"

•       " key = %lu, expected = %lu, returned = %lu\n",
  

•       table->magic, key, value, retval);
  

• break;
• case 1:
• printf("Not found: key = %lu, expected = %lu returned = %lu\n",

•       key, value, retval);
  

• break;
• case 0:
• if (value != retval)

•    printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
  

•    key, value, retval);
  

• break;
• default:
• printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",

•       retcode, key, value, retval);
  

• break;
• }

+}

+int main(void) +{

• HashTablePtr table;
• int i;
• printf("\n***** 256 consecutive integers ****\n");
• table = drmHashCreate();
• for (i = 0; i < 256; i++) drmHashInsert(table, i, i);
• for (i = 0; i < 256; i++) check_table(table, i, i);
• for (i = 256; i >= 0; i--) check_table(table, i, i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 1024 consecutive integers ****\n");
• table = drmHashCreate();
• for (i = 0; i < 1024; i++) drmHashInsert(table, i, i);
• for (i = 0; i < 1024; i++) check_table(table, i, i);
• for (i = 1024; i >= 0; i--) check_table(table, i, i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 1024 consecutive page addresses (4k pages) ****\n");
• table = drmHashCreate();
• for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, i);
• for (i = 0; i < 1024; i++) check_table(table, i*4096, i);
• for (i = 1024; i >= 0; i--) check_table(table, i*4096, i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 1024 random integers ****\n");
• table = drmHashCreate();
• srandom(0xbeefbeef);
• for (i = 0; i < 1024; i++) drmHashInsert(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 1024; i++) check_table(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 1024; i++) check_table(table, random(), i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 5000 random integers ****\n");
• table = drmHashCreate();
• srandom(0xbeefbeef);
• for (i = 0; i < 5000; i++) drmHashInsert(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 5000; i++) check_table(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 5000; i++) check_table(table, random(), i);
• compute_dist(table);
• drmHashDestroy(table);
• return 0;

+} diff --git a/xf86drmHash.c b/xf86drmHash.c index 82cbc2a..82512a8 100644 --- a/xf86drmHash.c +++ b/xf86drmHash.c @@ -71,11 +71,7 @@ #include <stdio.h> #include <stdlib.h>

-#define HASH_MAIN 0

-#if !HASH_MAIN -# include "xf86drm.h" -#endif +#include "xf86drm.h"

#define HASH_MAGIC 0xdeadbeef #define HASH_DEBUG 0 @@ -84,23 +80,6 @@ have to change the HashHash hashing function! */

-#if HASH_MAIN -#define HASH_ALLOC malloc -#define HASH_FREE free -#define HASH_RANDOM_DECL -#define HASH_RANDOM_INIT(seed) srandom(seed) -#define HASH_RANDOM random() -#define HASH_RANDOM_DESTROY -#else -#define HASH_ALLOC drmMalloc -#define HASH_FREE drmFree -#define HASH_RANDOM_DECL void *state -#define HASH_RANDOM_INIT(seed) state = drmRandomCreate(seed) -#define HASH_RANDOM drmRandom(state) -#define HASH_RANDOM_DESTROY drmRandomDestroy(state)

-#endif

typedef struct HashBucket { unsigned long key; void *value; @@ -118,14 +97,6 @@ typedef struct HashTable { HashBucketPtr p1; } HashTable, *HashTablePtr;

-#if HASH_MAIN -extern void *drmHashCreate(void); -extern int drmHashDestroy(void *t); -extern int drmHashLookup(void *t, unsigned long key, unsigned long *value); -extern int drmHashInsert(void *t, unsigned long key, unsigned long value); -extern int drmHashDelete(void *t, unsigned long key); -#endif

static unsigned long HashHash(unsigned long key) { unsigned long hash = 0; @@ -135,10 +106,10 @@ static unsigned long HashHash(unsigned long key) int i;

 if (!init) {

• HASH_RANDOM_DECL;
• HASH_RANDOM_INIT(37);
• for (i = 0; i < 256; i++) scatter[i] = HASH_RANDOM;
• HASH_RANDOM_DESTROY;

• void *state;
• state = drmRandomCreate(37);
• for (i = 0; i < 256; i++) scatter[i] = drmRandom(state);
• drmRandomDestroy(state); ++init; }

@@ -159,7 +130,7 @@ void *drmHashCreate(void) HashTablePtr table; int i;

• table = HASH_ALLOC(sizeof(*table));

• table = drmMalloc(sizeof(*table)); if (!table) return NULL; table->magic = HASH_MAGIC; table->entries = 0;

@@ -183,11 +154,11 @@ int drmHashDestroy(void *t) for (i = 0; i < HASH_SIZE; i++) { for (bucket = table->buckets[i]; bucket;) { next = bucket->next;

•    HASH_FREE(bucket);
  

•    drmFree(bucket);
   bucket = next;
  

  } }

• HASH_FREE(table);

• drmFree(table); return 0;

}

@@ -245,7 +216,7 @@ int drmHashInsert(void *t, unsigned long key, void *value)

 if (HashFind(table, key, &hash)) return 1; /* Already in table */

• bucket = HASH_ALLOC(sizeof(*bucket));

• bucket = drmMalloc(sizeof(*bucket)); if (!bucket) return -1; /* Error */ bucket->key = key; bucket->value = value;

@@ -270,7 +241,7 @@ int drmHashDelete(void *t, unsigned long key) if (!bucket) return 1; /* Not found */

 table->buckets[hash] = bucket->next;

• HASH_FREE(bucket);

• drmFree(bucket); return 0;

}

@@ -301,128 +272,3 @@ int drmHashFirst(void *t, unsigned long *key, void **value) table->p1 = table->buckets[0]; return drmHashNext(table, key, value); }

-#if HASH_MAIN -#define DIST_LIMIT 10 -static int dist[DIST_LIMIT];

-static void clear_dist(void) {

• int i;
• for (i = 0; i < DIST_LIMIT; i++) dist[i] = 0;

-}

-static int count_entries(HashBucketPtr bucket) -{

• int count = 0;
• for (; bucket; bucket = bucket->next) ++count;
• return count;

-}

-static void update_dist(int count) -{

• if (count >= DIST_LIMIT) ++dist[DIST_LIMIT-1];
• else ++dist[count];

-}

-static void compute_dist(HashTablePtr table) -{

• int i;
• HashBucketPtr bucket;
• printf("Entries = %ld, hits = %ld, partials = %ld, misses = %ld\n",

•   table->entries, table->hits, table->partials, table->misses);
  

• clear_dist();
• for (i = 0; i < HASH_SIZE; i++) {
• bucket = table->buckets[i];
• update_dist(count_entries(bucket));
• }
• for (i = 0; i < DIST_LIMIT; i++) {
• if (i != DIST_LIMIT-1) printf("%5d %10d\n", i, dist[i]);
• else printf("other %10d\n", dist[i]);
• }

-}

-static void check_table(HashTablePtr table,

• 	unsigned long key, unsigned long value)
  

-{

• unsigned long retval = 0;
• int retcode = drmHashLookup(table, key, &retval);
• switch (retcode) {
• case -1:
• printf("Bad magic = 0x%08lx:"

•       " key = %lu, expected = %lu, returned = %lu\n",
  

•       table->magic, key, value, retval);
  

• break;
• case 1:
• printf("Not found: key = %lu, expected = %lu returned = %lu\n",

•       key, value, retval);
  

• break;
• case 0:
• if (value != retval)

•    printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
  

•    key, value, retval);
  

• break;
• default:
• printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",

•       retcode, key, value, retval);
  

• break;
• }

-}

-int main(void) -{

• HashTablePtr table;
• int i;
• printf("\n***** 256 consecutive integers ****\n");
• table = drmHashCreate();
• for (i = 0; i < 256; i++) drmHashInsert(table, i, i);
• for (i = 0; i < 256; i++) check_table(table, i, i);
• for (i = 256; i >= 0; i--) check_table(table, i, i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 1024 consecutive integers ****\n");
• table = drmHashCreate();
• for (i = 0; i < 1024; i++) drmHashInsert(table, i, i);
• for (i = 0; i < 1024; i++) check_table(table, i, i);
• for (i = 1024; i >= 0; i--) check_table(table, i, i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 1024 consecutive page addresses (4k pages) ****\n");
• table = drmHashCreate();
• for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, i);
• for (i = 0; i < 1024; i++) check_table(table, i*4096, i);
• for (i = 1024; i >= 0; i--) check_table(table, i*4096, i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 1024 random integers ****\n");
• table = drmHashCreate();
• srandom(0xbeefbeef);
• for (i = 0; i < 1024; i++) drmHashInsert(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 1024; i++) check_table(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 1024; i++) check_table(table, random(), i);
• compute_dist(table);
• drmHashDestroy(table);
• printf("\n***** 5000 random integers ****\n");
• table = drmHashCreate();
• srandom(0xbeefbeef);
• for (i = 0; i < 5000; i++) drmHashInsert(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 5000; i++) check_table(table, random(), i);
• srandom(0xbeefbeef);
• for (i = 0; i < 5000; i++) check_table(table, random(), i);
• compute_dist(table);
• drmHashDestroy(table);
• return 0;

-} -#endif

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

Get the test from completely broken to working like a charm.

• Use the same variable type for both HashInsert and HashLookup.
• Use correct storage type for the HashLookup return value.
• Remove useless backward iteration of HashLookup(i).

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/hash.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/tests/hash.c b/tests/hash.c index d57d2dc..902919f 100644 --- a/tests/hash.c +++ b/tests/hash.c @@ -139,27 +139,27 @@ static void compute_dist(HashTablePtr table) static void check_table(HashTablePtr table, unsigned long key, unsigned long value)

I think we should use void* for value here.

{

• unsigned long retval = 0;
• int retcode = drmHashLookup(table, key, &retval);

• unsigned long *retval;
• int retcode = drmHashLookup(table, key, (void **)&retval);

I don't think this is correct. If the entry is found, it stores address to stack variable i in retval, which at this point in iteration incidentally contains the value we are looking for.

 switch (retcode) {
 case -1:

printf("Bad magic = 0x%08lx:" " key = %lu, expected = %lu, returned = %lu\n",

•       table->magic, key, value, retval);
  

•       table->magic, key, value, *retval);
  

  break; case 1:

• printf("Not found: key = %lu, expected = %lu returned = %lu\n",

•       key, value, retval);
  

• printf("Not found: key = %lu, expected = %lu, returned = %lu\n",

•       key, value, *retval);
  

  break; case 0:

• if (value != retval)

• if (value != *retval) printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",

•    key, value, retval);
  

•    key, value, *retval);
  

  break; default: printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",

•       retcode, key, value, retval);
  

•       retcode, key, value, *retval);
  

  break; }

} @@ -167,36 +167,33 @@ static void check_table(HashTablePtr table, int main(void) { HashTablePtr table;

• int i;

• unsigned long i;

  printf("\n***** 256 consecutive integers ****\n"); table = drmHashCreate();

• for (i = 0; i < 256; i++) drmHashInsert(table, i, i);

• for (i = 0; i < 256; i++) drmHashInsert(table, i, (void *)&i);

This changes the entries inserted. previously it would insert values [0, 256). Now it inserts address of i 256 times. I think we should change the inserted values to be different from the key (offset should be enough), to catch the kind of scenario this tests creates.

 for (i = 0; i < 256; i++) check_table(table, i, i);

• for (i = 256; i >= 0; i--) check_table(table, i, i); compute_dist(table); drmHashDestroy(table);

  printf("\n***** 1024 consecutive integers ****\n"); table = drmHashCreate();

• for (i = 0; i < 1024; i++) drmHashInsert(table, i, i);

• for (i = 0; i < 1024; i++) drmHashInsert(table, i, (void *)&i); for (i = 0; i < 1024; i++) check_table(table, i, i);

• for (i = 1024; i >= 0; i--) check_table(table, i, i); compute_dist(table); drmHashDestroy(table);

  printf("\n***** 1024 consecutive page addresses (4k pages) ****\n"); table = drmHashCreate();

• for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, i);

• for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, (void *)&i); for (i = 0; i < 1024; i++) check_table(table, i*4096, i);

• for (i = 1024; i >= 0; i--) check_table(table, i*4096, i); compute_dist(table); drmHashDestroy(table);

  printf("\n***** 1024 random integers ****\n"); table = drmHashCreate(); srandom(0xbeefbeef);

• for (i = 0; i < 1024; i++) drmHashInsert(table, random(), i);

• for (i = 0; i < 1024; i++) drmHashInsert(table, random(), (void *)&i); srandom(0xbeefbeef); for (i = 0; i < 1024; i++) check_table(table, random(), i); srandom(0xbeefbeef);

@@ -207,7 +204,7 @@ int main(void) printf("\n***** 5000 random integers ****\n"); table = drmHashCreate(); srandom(0xbeefbeef);

• for (i = 0; i < 5000; i++) drmHashInsert(table, random(), i);

• for (i = 0; i < 5000; i++) drmHashInsert(table, random(), (void *)&i); srandom(0xbeefbeef); for (i = 0; i < 5000; i++) check_table(table, random(), i); srandom(0xbeefbeef);

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/hash.c | 102 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 60 insertions(+), 42 deletions(-)

diff --git a/tests/hash.c b/tests/hash.c index 902919f..fa9264a 100644 --- a/tests/hash.c +++ b/tests/hash.c @@ -73,8 +73,8 @@

#include "xf86drm.h"

-#define HASH_SIZE 512 /* Good for about 100 entries */

• 		/* If you change this value, you probably
  

+#define HASH_SIZE 512 /* Good for about 100 entries */

•                            /* If you change this value, you probably
                                have to change the HashHash hashing
                                function! */
  
  

@@ -87,9 +87,9 @@ typedef struct HashBucket { typedef struct HashTable { unsigned long magic; unsigned long entries;

• unsigned long hits; /* At top of linked list */
• unsigned long partials; /* Not at top of linked list */
• unsigned long misses; /* Not in table */

• unsigned long hits; /* At top of linked list */
• unsigned long partials; /* Not at top of linked list */
• unsigned long misses; /* Not in table */ HashBucketPtr buckets[HASH_SIZE]; int p0; HashBucketPtr p1;

@@ -101,21 +101,25 @@ static int dist[DIST_LIMIT]; static void clear_dist(void) { int i;

• for (i = 0; i < DIST_LIMIT; i++) dist[i] = 0;

• for (i = 0; i < DIST_LIMIT; i++)

•    dist[i] = 0;
  

}

static int count_entries(HashBucketPtr bucket) {

• int count = 0;

• int count;

• for (; bucket; bucket = bucket->next) ++count;

• for (count = 0; bucket; bucket = bucket->next)

•    ++count;
  

I personally prefer to initialize early, especially since it's not for-loop iterating variable, but I don't insist.

Reviewed-by: Jan Vesely jan.vesely@rutgers.edu

 return count;

}

static void update_dist(int count) {

• if (count >= DIST_LIMIT) ++dist[DIST_LIMIT-1];
• else ++dist[count];

• if (count >= DIST_LIMIT)

•    ++dist[DIST_LIMIT-1];
  

• else

•    ++dist[count];
  

}

static void compute_dist(HashTablePtr table) @@ -124,43 +128,45 @@ static void compute_dist(HashTablePtr table) HashBucketPtr bucket;

 printf("Entries = %ld, hits = %ld, partials = %ld, misses = %ld\n",

•   table->entries, table->hits, table->partials, table->misses);
  

•      table->entries, table->hits, table->partials, table->misses);
  

  clear_dist(); for (i = 0; i < HASH_SIZE; i++) {

• bucket = table->buckets[i];
• update_dist(count_entries(bucket));

•    bucket = table->buckets[i];
  

•    update_dist(count_entries(bucket));
  

  } for (i = 0; i < DIST_LIMIT; i++) {

• if (i != DIST_LIMIT-1) printf("%5d %10d\n", i, dist[i]);
• else printf("other %10d\n", dist[i]);

•    if (i != DIST_LIMIT-1)
  

•        printf("%5d %10d\n", i, dist[i]);
  

•    else
  

•        printf("other %10d\n", dist[i]);
  

  }

}

static void check_table(HashTablePtr table,

• 	unsigned long key, unsigned long value)
  

•                    unsigned long key, unsigned long value)
  

{ unsigned long *retval; int retcode = drmHashLookup(table, key, (void **)&retval);

 switch (retcode) {
 case -1:

• printf("Bad magic = 0x%08lx:"

•       " key = %lu, expected = %lu, returned = %lu\n",
  

•       table->magic, key, value, *retval);
  

• break;

•    printf("Bad magic = 0x%08lx:"
  

•           " key = %lu, expected = %lu, returned = %lu\n",
  

•           table->magic, key, value, *retval);
  

•    break;
  

  case 1:

• printf("Not found: key = %lu, expected = %lu, returned = %lu\n",

•       key, value, *retval);
  

• break;

•    printf("Not found: key = %lu, expected = %lu, returned = %lu\n",
  

•           key, value, *retval);
  

•    break;
  

  case 0:

• if (value != *retval)

•    printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
  

•    key, value, *retval);
  

• break;

•    if (value != *retval)
  

•        printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
  

•               key, value, *retval);
  

•    break;
  

  default:

• printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",

•       retcode, key, value, *retval);
  

• break;

•    printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n",
  

•           retcode, key, value, *retval);
  

•    break;
  

  }

}

@@ -171,44 +177,56 @@ int main(void)

 printf("\n***** 256 consecutive integers ****\n");
 table = drmHashCreate();

• for (i = 0; i < 256; i++) drmHashInsert(table, i, (void *)&i);
• for (i = 0; i < 256; i++) check_table(table, i, i);

• for (i = 0; i < 256; i++)

•    drmHashInsert(table, i, (void *)&i);
  

• for (i = 0; i < 256; i++)

•    check_table(table, i, i);
  

  compute_dist(table); drmHashDestroy(table);

  printf("\n***** 1024 consecutive integers ****\n"); table = drmHashCreate();

• for (i = 0; i < 1024; i++) drmHashInsert(table, i, (void *)&i);
• for (i = 0; i < 1024; i++) check_table(table, i, i);

• for (i = 0; i < 1024; i++)

•    drmHashInsert(table, i, (void *)&i);
  

• for (i = 0; i < 1024; i++)

•    check_table(table, i, i);
  

  compute_dist(table); drmHashDestroy(table);

  printf("\n***** 1024 consecutive page addresses (4k pages) ****\n"); table = drmHashCreate();

• for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, (void *)&i);
• for (i = 0; i < 1024; i++) check_table(table, i*4096, i);

• for (i = 0; i < 1024; i++)

•    drmHashInsert(table, i*4096, (void *)&i);
  

• for (i = 0; i < 1024; i++)

•    check_table(table, i*4096, i);
  

  compute_dist(table); drmHashDestroy(table);

  printf("\n***** 1024 random integers ****\n"); table = drmHashCreate(); srandom(0xbeefbeef);

• for (i = 0; i < 1024; i++) drmHashInsert(table, random(), (void *)&i);

• for (i = 0; i < 1024; i++)

•    drmHashInsert(table, random(), (void *)&i);
  

  srandom(0xbeefbeef);

• for (i = 0; i < 1024; i++) check_table(table, random(), i);

• for (i = 0; i < 1024; i++)

•    check_table(table, random(), i);
  

  srandom(0xbeefbeef);

• for (i = 0; i < 1024; i++) check_table(table, random(), i);

• for (i = 0; i < 1024; i++)

•    check_table(table, random(), i);
  

  compute_dist(table); drmHashDestroy(table);

  printf("\n***** 5000 random integers ****\n"); table = drmHashCreate(); srandom(0xbeefbeef);

• for (i = 0; i < 5000; i++) drmHashInsert(table, random(), (void *)&i);

• for (i = 0; i < 5000; i++)

•    drmHashInsert(table, random(), (void *)&i);
  

  srandom(0xbeefbeef);

• for (i = 0; i < 5000; i++) check_table(table, random(), i);

• for (i = 0; i < 5000; i++)

•    check_table(table, random(), i);
  

  srandom(0xbeefbeef);

• for (i = 0; i < 5000; i++) check_table(table, random(), i);

• for (i = 0; i < 5000; i++)

•    check_table(table, random(), i);
  

  compute_dist(table); drmHashDestroy(table);

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

... and wire up to `make check' now that it's useful.

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/Makefile.am | 12 +++++++----- tests/hash.c | 26 +++++++++++++++----------- 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am index ea826b5..392abf5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,13 +29,15 @@ LDADD = $(top_builddir)/libdrm.la

check_PROGRAMS = \ dristat \

• drmstat \
• hash

• drmstat

if HAVE_NOUVEAU SUBDIRS += nouveau endif

+TESTS = \

• hash

if HAVE_LIBUDEV

check_LTLIBRARIES = libdrmtest.la @@ -53,7 +55,7 @@ XFAIL_TESTS = \ auth \ lock

-TESTS = \ +TESTS += \ openclose \ getversion \ getclient \ @@ -62,6 +64,6 @@ TESTS = \ updatedraw \ name_from_fd

-check_PROGRAMS += $(TESTS)

endif

+check_PROGRAMS += $(TESTS) diff --git a/tests/hash.c b/tests/hash.c index fa9264a..46f52f8 100644 --- a/tests/hash.c +++ b/tests/hash.c @@ -142,7 +142,7 @@ static void compute_dist(HashTablePtr table) } }

-static void check_table(HashTablePtr table, +static int check_table(HashTablePtr table, unsigned long key, unsigned long value) { unsigned long *retval; @@ -159,28 +159,32 @@ static void check_table(HashTablePtr table, key, value, *retval); break; case 0:

•    if (value != *retval)
  

•    if (value != *retval) {
         printf("Bad value: key = %lu, expected = %lu, returned = %lu\n",
                key, value, *retval);
  

•        retcode = -1;
  

•    }
     break;
  

  default: printf("Bad retcode = %d: key = %lu, expected = %lu, returned = %lu\n", retcode, key, value, *retval); break; }
• return retcode;

}

int main(void) {

• HashTablePtr table;
• unsigned long i;

• HashTablePtr table;

• unsigned long i;

• int ret = 0;

  printf("\n***** 256 consecutive integers ****\n"); table = drmHashCreate(); for (i = 0; i < 256; i++) drmHashInsert(table, i, (void *)&i); for (i = 0; i < 256; i++)

•    check_table(table, i, i);
  

•    ret = check_table(table, i, i) && ret;
  

  compute_dist(table); drmHashDestroy(table);

@@ -189,7 +193,7 @@ int main(void) for (i = 0; i < 1024; i++) drmHashInsert(table, i, (void *)&i); for (i = 0; i < 1024; i++)

•    check_table(table, i, i);
  

•    ret = check_table(table, i, i) && ret;
  

  compute_dist(table); drmHashDestroy(table);

@@ -198,7 +202,7 @@ int main(void) for (i = 0; i < 1024; i++) drmHashInsert(table, i*4096, (void *)&i); for (i = 0; i < 1024; i++)

•    check_table(table, i*4096, i);
  

•    ret = check_table(table, i*4096, i) && ret;
  

  compute_dist(table); drmHashDestroy(table);

@@ -209,10 +213,10 @@ int main(void) drmHashInsert(table, random(), (void *)&i); srandom(0xbeefbeef); for (i = 0; i < 1024; i++)

•    check_table(table, random(), i);
  

•    ret = check_table(table, random(), i) && ret;
  

  srandom(0xbeefbeef); for (i = 0; i < 1024; i++)

•    check_table(table, random(), i);
  

•    ret = check_table(table, random(), i) && ret;
  

  compute_dist(table); drmHashDestroy(table);

@@ -223,10 +227,10 @@ int main(void) drmHashInsert(table, random(), (void *)&i); srandom(0xbeefbeef); for (i = 0; i < 5000; i++)

•    check_table(table, random(), i);
  

•    ret = check_table(table, random(), i) && ret;
  

  srandom(0xbeefbeef); for (i = 0; i < 5000; i++)

•    check_table(table, random(), i);
  

•    ret = check_table(table, random(), i) && ret;
  

  compute_dist(table); drmHashDestroy(table);

Reviewed-by: Jan Vesely jan.vesely@rutgesr.edu

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

With follow up commits we can clear it up and wire to make check

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/Makefile.am | 3 +- tests/random.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ xf86drmRandom.c | 67 ++-------------------------- 3 files changed, 132 insertions(+), 65 deletions(-) create mode 100644 tests/random.c

diff --git a/tests/Makefile.am b/tests/Makefile.am index 392abf5..9b13b2e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,7 +29,8 @@ LDADD = $(top_builddir)/libdrm.la

check_PROGRAMS = \ dristat \

• drmstat

• drmstat \
• random

if HAVE_NOUVEAU SUBDIRS += nouveau diff --git a/tests/random.c b/tests/random.c new file mode 100644 index 0000000..6dc8386 --- /dev/null +++ b/tests/random.c @@ -0,0 +1,127 @@ +/* xf86drmRandom.c -- "Minimal Standard" PRNG Implementation

• • Created: Mon Apr 19 08:28:13 1999 by faith@precisioninsight.com
• • Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
• • All Rights Reserved.
• • Permission is hereby granted, free of charge, to any person obtaining a
• • copy of this software and associated documentation files (the "Software"),
• • to deal in the Software without restriction, including without limitation
• • the rights to use, copy, modify, merge, publish, distribute, sublicense,
• • and/or sell copies of the Software, and to permit persons to whom the
• • Software is furnished to do so, subject to the following conditions:
• • The above copyright notice and this permission notice (including the next
• • paragraph) shall be included in all copies or substantial portions of the
• • Software.
• • THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
• • IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
• • FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
• • PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
• • OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
• • ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
• • DEALINGS IN THE SOFTWARE.
• • Authors: Rickard E. (Rik) Faith faith@valinux.com
• • DESCRIPTION
• • This file contains a simple, straightforward implementation of the Park
• • & Miller "Minimal Standard" PRNG [PM88, PMS93], which is a Lehmer
• • multiplicative linear congruential generator (MLCG) with a period of
• • 2^31-1.
• • This implementation is intended to provide a reliable, portable PRNG
• • that is suitable for testing a hash table implementation and for
• • implementing skip lists.
• • FUTURE ENHANCEMENTS
• • If initial seeds are not selected randomly, two instances of the PRNG
• • can be correlated. [Knuth81, pp. 32-33] describes a shuffling technique
• • that can eliminate this problem.
• • If PRNGs are used for simulation, the period of the current
• • implementation may be too short. [LE88] discusses methods of combining
• • MLCGs to produce much longer periods, and suggests some alternative
• • values for A and M. [LE90 and Sch92] also provide information on
• • long-period PRNGs.
• • REFERENCES
• • [Knuth81] Donald E. Knuth. The Art of Computer Programming. Volume 2:
• • Seminumerical Algorithms. Reading, Massachusetts: Addison-Wesley, 1981.
• • [LE88] Pierre L'Ecuyer. "Efficient and Portable Combined Random Number
• • Generators". CACM 31(6), June 1988, pp. 742-774.
• • [LE90] Pierre L'Ecuyer. "Random Numbers for Simulation". CACM 33(10,
• • October 1990, pp. 85-97.
• • [PM88] Stephen K. Park and Keith W. Miller. "Random Number Generators:
• • Good Ones are Hard to Find". CACM 31(10), October 1988, pp. 1192-1201.
• • [Sch92] Bruce Schneier. "Pseudo-Ransom Sequence Generator for 32-Bit
• • CPUs". Dr. Dobb's Journal 17(2), February 1992, pp. 34, 37-38, 40.
• • [PMS93] Stephen K. Park, Keith W. Miller, and Paul K. Stockmeyer. In
• • "Technical Correspondence: Remarks on Choosing and Implementing Random
• • Number Generators". CACM 36(7), July 1993, pp. 105-110.
• */

+#include <stdio.h> +#include <stdlib.h>

+#include "xf86drm.h"

+typedef struct RandomState {

• unsigned long magic;
• unsigned long a;
• unsigned long m;
• unsigned long q; /* m div a */
• unsigned long r; /* m mod a */
• unsigned long check;
• unsigned long seed;

+} RandomState;

I think this should also go to an internal header. Otherwise lgtm

+static void check_period(unsigned long seed) +{

• unsigned long count = 0;
• unsigned long initial;
• void *state;
• state = drmRandomCreate(seed);
• initial = drmRandom(state);
• ++count;
• while (initial != drmRandom(state)) {
• if (!++count) break;
• }
• printf("With seed of %10lu, period = %10lu (0x%08lx)\n",

•   seed, count, count);
  

• drmRandomDestroy(state);

+}

+int main(void) +{

• RandomState *state;
• int i;
• unsigned long rand;
• state = drmRandomCreate(1);
• for (i = 0; i < 10000; i++) {
• rand = drmRandom(state);
• }
• printf("After 10000 iterations: %lu (%lu expected): %s\n",

•   rand, state->check,
  

•   rand - state->check ? "*INCORRECT*" : "CORRECT");
  

• drmRandomDestroy(state);
• printf("Checking periods...\n");
• check_period(1);
• check_period(2);
• check_period(31415926);
• return 0;

+} diff --git a/xf86drmRandom.c b/xf86drmRandom.c index 94922ad..39f3c52 100644 --- a/xf86drmRandom.c +++ b/xf86drmRandom.c @@ -74,23 +74,11 @@ #include <stdio.h> #include <stdlib.h>

-#define RANDOM_MAIN 0

-#if !RANDOM_MAIN -# include "xf86drm.h" -#endif +#include "xf86drm.h"

#define RANDOM_MAGIC 0xfeedbeef #define RANDOM_DEBUG 0

-#if RANDOM_MAIN -#define RANDOM_ALLOC malloc -#define RANDOM_FREE free -#else -#define RANDOM_ALLOC drmMalloc -#define RANDOM_FREE drmFree -#endif

typedef struct RandomState { unsigned long magic; unsigned long a; @@ -101,18 +89,11 @@ typedef struct RandomState { unsigned long seed; } RandomState;

-#if RANDOM_MAIN -extern void *drmRandomCreate(unsigned long seed); -extern int drmRandomDestroy(void *state); -extern unsigned long drmRandom(void *state); -extern double drmRandomDouble(void *state); -#endif

void *drmRandomCreate(unsigned long seed) { RandomState *state;

• state = RANDOM_ALLOC(sizeof(*state));

• state = drmMalloc(sizeof(*state)); if (!state) return NULL; state->magic = RANDOM_MAGIC;

#if 0 @@ -140,7 +121,7 @@ void *drmRandomCreate(unsigned long seed)

int drmRandomDestroy(void *state) {

• RANDOM_FREE(state);

• drmFree(state); return 0;

}

@@ -164,45 +145,3 @@ double drmRandomDouble(void *state)

 return (double)drmRandom(state)/(double)s->m;

}

-#if RANDOM_MAIN -static void check_period(unsigned long seed) -{

• unsigned long count = 0;
• unsigned long initial;
• void *state;
• state = drmRandomCreate(seed);
• initial = drmRandom(state);
• ++count;
• while (initial != drmRandom(state)) {
• if (!++count) break;
• }
• printf("With seed of %10lu, period = %10lu (0x%08lx)\n",

•   seed, count, count);
  

• drmRandomDestroy(state);

-}

-int main(void) -{

• RandomState *state;
• int i;
• unsigned long rand;
• state = drmRandomCreate(1);
• for (i = 0; i < 10000; i++) {
• rand = drmRandom(state);
• }
• printf("After 10000 iterations: %lu (%lu expected): %s\n",

•   rand, state->check,
  

•   rand - state->check ? "*INCORRECT*" : "CORRECT");
  

• drmRandomDestroy(state);
• printf("Checking periods...\n");
• check_period(1);
• check_period(2);
• check_period(31415926);
• return 0;

-} -#endif

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

... and wire it up to make check

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

tests/Makefile.am | 6 +++--- tests/random.c | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am index 9b13b2e..0603241 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,15 +29,15 @@ LDADD = $(top_builddir)/libdrm.la

check_PROGRAMS = \ dristat \

• drmstat \
• random

• drmstat

if HAVE_NOUVEAU SUBDIRS += nouveau endif

TESTS = \

• hash

• hash \
• random

if HAVE_LIBUDEV

diff --git a/tests/random.c b/tests/random.c index 6dc8386..6af7d33 100644 --- a/tests/random.c +++ b/tests/random.c @@ -107,15 +107,17 @@ int main(void) { RandomState *state; int i;

• int ret; unsigned long rand;

  state = drmRandomCreate(1); for (i = 0; i < 10000; i++) { rand = drmRandom(state); }

• ret = rand - state->check;

Since you are touching this line, I think rand != state->check would be more readable.

 printf("After 10000 iterations: %lu (%lu expected): %s\n",
  rand, state->check,

•   rand - state->check ? "*INCORRECT*" : "CORRECT");
  

•   ret ? "*INCORRECT*" : "CORRECT");
  

  drmRandomDestroy(state);

  printf("Checking periods...\n");

@@ -123,5 +125,5 @@ int main(void) check_period(2); check_period(31415926);

• return 0;

• return ret;

}

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

... and remove the useless SL_DEBUG and RANDOM_DEBUG

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

xf86drmHash.c | 5 ++--- xf86drmRandom.c | 1 - xf86drmSL.c | 1 - 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/xf86drmHash.c b/xf86drmHash.c index 82512a8..12baa62 100644 --- a/xf86drmHash.c +++ b/xf86drmHash.c @@ -74,7 +74,6 @@ #include "xf86drm.h"

#define HASH_MAGIC 0xdeadbeef -#define HASH_DEBUG 0 #define HASH_SIZE 512 /* Good for about 100 entries */ /* If you change this value, you probably have to change the HashHash hashing @@ -119,7 +118,7 @@ static unsigned long HashHash(unsigned long key) }

 hash %= HASH_SIZE;

-#if HASH_DEBUG +#if DEBUG printf( "Hash(%d) = %d\n", key, hash); #endif return hash; @@ -222,7 +221,7 @@ int drmHashInsert(void *t, unsigned long key, void *value) bucket->value = value; bucket->next = table->buckets[hash]; table->buckets[hash] = bucket; -#if HASH_DEBUG +#if DEBUG printf("Inserted %d at %d/%p\n", key, hash, bucket); #endif return 0; /* Added to table */ diff --git a/xf86drmRandom.c b/xf86drmRandom.c index 39f3c52..2177d27 100644 --- a/xf86drmRandom.c +++ b/xf86drmRandom.c @@ -77,7 +77,6 @@ #include "xf86drm.h"

#define RANDOM_MAGIC 0xfeedbeef -#define RANDOM_DEBUG 0

typedef struct RandomState { unsigned long magic; diff --git a/xf86drmSL.c b/xf86drmSL.c index acddb54..9bbf8fb 100644 --- a/xf86drmSL.c +++ b/xf86drmSL.c @@ -53,7 +53,6 @@ #define SL_ENTRY_MAGIC 0x00fab1edLU #define SL_FREED_MAGIC 0xdecea5edLU #define SL_MAX_LEVEL 16 -#define SL_DEBUG 0 #define SL_RANDOM_SEED 0xc01055a1LU

#if SL_MAIN

Reviewed-by: Jan Vesely jan.vesely@rutgers.edu

On Sun, 2015-03-22 at 22:03 +0000, Emil Velikov wrote:

The valies are unsigned long, thus we should use %lu.

Signed-off-by: Emil Velikov emil.l.velikov@gmail.com

xf86drmHash.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xf86drmHash.c b/xf86drmHash.c index 12baa62..887d6a7 100644 --- a/xf86drmHash.c +++ b/xf86drmHash.c @@ -119,7 +119,7 @@ static unsigned long HashHash(unsigned long key)

 hash %= HASH_SIZE;

#if DEBUG

• printf( "Hash(%d) = %d\n", key, hash);

• printf( "Hash(%lu) = %lu\n", key, hash);

#endif return hash; } @@ -221,8 +221,9 @@ int drmHashInsert(void *t, unsigned long key, void *value) bucket->value = value; bucket->next = table->buckets[hash]; table->buckets[hash] = bucket; -#if DEBUG printf("Inserted %d at %d/%p\n", key, hash, bucket); +#if DEBUG

• printf("Inserted %lu at %lu/%p\n", key, hash, bucket);

#endif

^^ This change looks funny in the patch. I assume you just replace the printf call? If that's so LGTM Reviewed-by: Jan Vesely jan.vesely@rutgers.edu

 return 0;			/* Added to table */

}