more registry code cleanup and re-organization for better management

diff --git a/src/avl.h b/src/avl.h
index c15291caaecd82d7b2fa5cd9c36410a2f9272c0a..de4c24bbac131e007c9c380bdf297b1dcb2876df 100644 (file)
--- a/src/avl.h
+++ b/src/avl.h
@@ -56,8 +56,8 @@ typedef struct avl_tree_lock {
  * a is linked directly to the tree, so it has to
  * be properly allocated by the caller.
  */
-avl *avl_insert_lock(avl_tree_lock *t, avl *a);
-avl *avl_insert(avl_tree *t, avl *a);
+avl *avl_insert_lock(avl_tree_lock *t, avl *a) __attribute__((returns_nonnull));
+avl *avl_insert(avl_tree *t, avl *a) __attribute__((returns_nonnull));
 
 /* Remove an element a from the AVL tree t
  * returns a pointer to the removed element

diff --git a/src/common.h b/src/common.h
index 0fc4e820dcffb6dafb474d689e247c4325b36690..c4255db059e0f19669422af45ce39d7fc05e7299 100644 (file)
--- a/src/common.h
+++ b/src/common.h
@@ -200,10 +200,10 @@ extern void *mallocz_int(const char *file, const char *function, const unsigned
 extern void *reallocz_int(const char *file, const char *function, const unsigned long line, void *ptr, size_t size);
 extern void freez_int(const char *file, const char *function, const unsigned long line, void *ptr);
 #else
-extern char *strdupz(const char *s) __attribute__((returns_nonnull));
-extern void *callocz(size_t nmemb, size_t size) __attribute__((returns_nonnull));
-extern void *mallocz(size_t size) __attribute__((returns_nonnull));
-extern void *reallocz(void *ptr, size_t size) __attribute__((returns_nonnull));
+extern char *strdupz(const char *s) __attribute__((returns_nonnull, malloc));
+extern void *callocz(size_t nmemb, size_t size) __attribute__((returns_nonnull, malloc));
+extern void *mallocz(size_t size) __attribute__((returns_nonnull, malloc));
+extern void *reallocz(void *ptr, size_t size) __attribute__((returns_nonnull, malloc));
 extern void freez(void *ptr);
 #endif
 

diff --git a/src/registry.h b/src/registry.h
index 19ed802cb326f437b3888ccc456604c82e221756..cb7fb9f5e699ce99897e4018aef40a2f7b3ae75e 100644 (file)
--- a/src/registry.h
+++ b/src/registry.h
@@ -39,6 +39,7 @@
 // 3. lower memory requirements
 //
 //    - embed avl structures directly into registry objects, instead of DICTIONARY
+//      [DONE for PERSON_URLs, PENDING for MACHINE_URLs]
 //    - store GUIDs in memory as UUID instead of char *
 //    - do not track persons using the demo machines only
 //      (i.e. start tracking them only when they access a non-demo machine)

diff --git a/src/registry_init.c b/src/registry_init.c
index 1a086b0f72cc119f82d03ed1e7014e21ea450b83..fb61acd0808e673c646afba3d8abf6183a405d1d 100644 (file)
--- a/src/registry_init.c
+++ b/src/registry_init.c
@@ -80,7 +80,7 @@ int registry_init(void) {
 
     return 0;
 }
-/*
+
 void registry_free(void) {
     if(!registry.enabled) return;
 
@@ -89,32 +89,7 @@ void registry_free(void) {
 
     while(registry.persons->values_index.root) {
         REGISTRY_PERSON *p = ((NAME_VALUE *)registry.persons->values_index.root)->value;
-
-        // fprintf(stderr, "\nPERSON: '%s', first: %u, last: %u, usages: %u\n", p->guid, p->first_t, p->last_t, p->usages);
-
-        while(p->person_urls->values_index.root) {
-            REGISTRY_PERSON_URL *pu = ((NAME_VALUE *)p->person_urls->values_index.root)->value;
-
-            // fprintf(stderr, "\tURL: '%s', first: %u, last: %u, usages: %u, flags: 0x%02x\n", pu->url->url, pu->first_t, pu->last_t, pu->usages, pu->flags);
-
-            debug(D_REGISTRY, "Registry: deleting url '%s' from person '%s'", pu->url->url, p->guid);
-            dictionary_del(p->person_urls, pu->url->url);
-
-            debug(D_REGISTRY, "Registry: unlinking url '%s' from person", pu->url->url);
-            registry_url_unlink(pu->url);
-
-            debug(D_REGISTRY, "Registry: freeing person url");
-            freez(pu);
-        }
-
-        debug(D_REGISTRY, "Registry: deleting person '%s' from persons registry", p->guid);
-        dictionary_del(registry.persons, p->guid);
-
-        debug(D_REGISTRY, "Registry: destroying URL dictionary of person '%s'", p->guid);
-        dictionary_destroy(p->person_urls);
-
-        debug(D_REGISTRY, "Registry: freeing person '%s'", p->guid);
-        freez(p);
+        registry_person_del(p);
     }
 
     while(registry.machines->values_index.root) {
@@ -158,4 +133,4 @@ void registry_free(void) {
     debug(D_REGISTRY, "Registry: destroying machines dictionary");
     dictionary_destroy(registry.machines);
 }
-*/
+

diff --git a/src/registry_internals.c b/src/registry_internals.c
index e3cd04737d476c4061ae6d30049a29cba577a73b..d32d549e28352e9a87365068d6fc173e6945b4ad 100644 (file)
--- a/src/registry_internals.c
+++ b/src/registry_internals.c
@@ -59,7 +59,17 @@ static inline char *registry_fix_machine_name(char *name, size_t *len) {
 }
 
 static inline char *registry_fix_url(char *url, size_t *len) {
-    return registry_fix_machine_name(url, len);
+    size_t l = 0;
+    char *s = registry_fix_machine_name(url, &l);
+
+    // protection from too big URLs
+    if(l > registry.max_url_length) {
+        l = registry.max_url_length;
+        s[l] = '\0';
+    }
+
+    if(len) *len = l;
+    return s;
 }
 
 
@@ -115,7 +125,7 @@ REGISTRY_PERSON_URL *registry_verify_request(char *person_guid, char *machine_gu
     }
     if(pp) *pp = p;
 
-    REGISTRY_PERSON_URL *pu = registry_person_url_find(p, url);
+    REGISTRY_PERSON_URL *pu = registry_person_url_index_find(p, url);
     if(!pu) {
         info("Registry Request Verification: URL not found for person, person: '%s', machine '%s', url '%s'", person_guid, machine_guid, url);
         return NULL;
@@ -154,7 +164,7 @@ REGISTRY_PERSON *registry_request_access(char *person_guid, char *machine_guid,
 }
 
 REGISTRY_PERSON *registry_request_delete(char *person_guid, char *machine_guid, char *url, char *delete_url, time_t when) {
-    (void)when;
+    (void) when;
 
     REGISTRY_PERSON *p = NULL;
     REGISTRY_MACHINE *m = NULL;
@@ -166,23 +176,20 @@ REGISTRY_PERSON *registry_request_delete(char *person_guid, char *machine_guid,
 
     // make sure the user is not deleting the url it uses
     if(!strcmp(delete_url, pu->url->url)) {
-        info("Registry Delete Request: delete URL is the one currently accessed, person: '%s', machine '%s', url '%s', delete url '%s'", p->guid, m->guid, pu->url->url, delete_url);
+        info("Registry Delete Request: delete URL is the one currently accessed, person: '%s', machine '%s', url '%s', delete url '%s'"
+             , p->guid, m->guid, pu->url->url, delete_url);
         return NULL;
     }
 
-    REGISTRY_PERSON_URL *dpu = registry_person_url_find(p, delete_url);
+    REGISTRY_PERSON_URL *dpu = registry_person_url_index_find(p, delete_url);
     if(!dpu) {
-        info("Registry Delete Request: URL not found for person: '%s', machine '%s', url '%s', delete url '%s'", p->guid, m->guid, pu->url->url, delete_url);
+        info("Registry Delete Request: URL not found for person: '%s', machine '%s', url '%s', delete url '%s'", p->guid
+             , m->guid, pu->url->url, delete_url);
         return NULL;
     }
 
     registry_log('D', p, m, pu->url, dpu->url->url);
-
-    registry_person_url_del(p, dpu);
-
-    registry_url_unlink(dpu->url);
-
-    freez(dpu);
+    registry_person_unlink_from_url(p, dpu);
 
     return p;
 }

diff --git a/src/registry_person.c b/src/registry_person.c
index 1ff8fc639ecc3b7ee26035e32a5f68238522c416..5f9099c9aa6995eb2bc761910c31865a680393d9 100644 (file)
--- a/src/registry_person.c
+++ b/src/registry_person.c
@@ -1,7 +1,7 @@
 #include "registry_internals.h"
 
 // ----------------------------------------------------------------------------
-// PERSON_URL
+// PERSON_URL INDEX
 
 int person_url_compare(void *a, void *b) {
     register uint32_t hash1 = ((REGISTRY_PERSON_URL *)a)->url->hash;
@@ -12,11 +12,8 @@ int person_url_compare(void *a, void *b) {
     else return strcmp(((REGISTRY_PERSON_URL *)a)->url->url, ((REGISTRY_PERSON_URL *)b)->url->url);
 }
 
-#define registry_person_url_index_add(person, rc) (REGISTRY_PERSON_URL *)avl_insert(&((person)->person_urls), (avl *)(rc))
-#define registry_person_url_index_del(person, rc) (REGISTRY_PERSON_URL *)avl_remove(&((person)->person_urls), (avl *)(rc))
-
-REGISTRY_PERSON_URL *registry_person_url_find(REGISTRY_PERSON *p, const char *url) {
-    debug(D_REGISTRY, "Registry: registry_person_url_find('%s', '%s')", p->guid, url);
+inline REGISTRY_PERSON_URL *registry_person_url_index_find(REGISTRY_PERSON *p, const char *url) {
+    debug(D_REGISTRY, "Registry: registry_person_url_index_find('%s', '%s')", p->guid, url);
 
     char buf[sizeof(REGISTRY_URL) + strlen(url)];
 
@@ -30,6 +27,29 @@ REGISTRY_PERSON_URL *registry_person_url_find(REGISTRY_PERSON *p, const char *ur
     return pu;
 }
 
+inline REGISTRY_PERSON_URL *registry_person_url_index_add(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) {
+    debug(D_REGISTRY, "Registry: registry_person_url_index_add('%s', '%s')", p->guid, pu->url->url);
+    REGISTRY_PERSON_URL *tpu = (REGISTRY_PERSON_URL *)avl_insert(&(p->person_urls), (avl *)(pu));
+    if(tpu != pu)
+        error("Registry: registry_person_url_index_add('%s', '%s') already exists as '%s'", p->guid, pu->url->url, tpu->url->url);
+
+    return tpu;
+}
+
+inline REGISTRY_PERSON_URL *registry_person_url_index_del(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) {
+    debug(D_REGISTRY, "Registry: registry_person_url_index_del('%s', '%s')", p->guid, pu->url->url);
+    REGISTRY_PERSON_URL *tpu = (REGISTRY_PERSON_URL *)avl_remove(&(p->person_urls), (avl *)(pu));
+    if(!tpu)
+        error("Registry: registry_person_url_index_del('%s', '%s') deleted nothing", p->guid, pu->url->url);
+    else if(tpu != pu)
+        error("Registry: registry_person_url_index_del('%s', '%s') deleted wrong URL '%s'", p->guid, pu->url->url, tpu->url->url);
+
+    return tpu;
+}
+
+// ----------------------------------------------------------------------------
+// PERSON_URL
+
 REGISTRY_PERSON_URL *registry_person_url_allocate(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when) {
     debug(D_REGISTRY, "registry_person_url_allocate('%s', '%s', '%s'): allocating %zu bytes", p->guid, m->guid, u->url, sizeof(REGISTRY_PERSON_URL) + namelen);
 
@@ -53,22 +73,47 @@ REGISTRY_PERSON_URL *registry_person_url_allocate(REGISTRY_PERSON *p, REGISTRY_M
     registry.persons_urls_memory += sizeof(REGISTRY_PERSON_URL) + namelen;
 
     debug(D_REGISTRY, "registry_person_url_allocate('%s', '%s', '%s'): indexing URL in person", p->guid, m->guid, u->url);
-    registry_person_url_index_add(p, pu);
-
-    registry_url_link(u);
+    REGISTRY_PERSON_URL *tpu = registry_person_url_index_add(p, pu);
+    if(tpu != pu) {
+        error("Registry: Attempted to add duplicate person url '%s' with name '%s' to person '%s'", u->url, name, p->guid);
+        free(pu);
+        pu = tpu;
+    }
+    else
+        registry_url_link(u);
 
     return pu;
 }
 
+void registry_person_url_free(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) {
+    debug(D_REGISTRY, "registry_person_url_free('%s', '%s')", p->guid, pu->url->url);
+
+    REGISTRY_PERSON_URL *tpu = registry_person_url_index_del(p, pu);
+    if(tpu) {
+        registry_url_unlink(tpu->url);
+        tpu->machine->links--;
+        registry.persons_urls_memory -= sizeof(REGISTRY_PERSON_URL) + strlen(tpu->machine_name);
+        freez(tpu);
+    }
+}
+
 // this function is needed to change the name of a PERSON_URL
 REGISTRY_PERSON_URL *registry_person_url_reallocate(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when, REGISTRY_PERSON_URL *pu) {
     debug(D_REGISTRY, "registry_person_url_reallocate('%s', '%s', '%s'): allocating %zu bytes", p->guid, m->guid, u->url, sizeof(REGISTRY_PERSON_URL) + namelen);
 
+    // keep a backup
+    REGISTRY_PERSON_URL pu2 = {
+            .first_t = pu->first_t,
+            .last_t = pu->last_t,
+            .usages = pu->usages,
+            .flags = pu->flags,
+            .machine = pu->machine,
+            .machine_name = ""
+    };
+
     // remove the existing one from the index
-    registry_person_url_index_del(p, pu);
-    registry_url_unlink(pu->url);
-    pu->machine->links--;
-    registry.persons_urls_memory -= sizeof(REGISTRY_PERSON_URL) + strlen(pu->machine_name);
+    registry_person_url_free(p, pu);
+    pu = &pu2;
 
     // allocate a new one
     REGISTRY_PERSON_URL *tpu = registry_person_url_allocate(p, m, u, name, namelen, when);
@@ -77,16 +122,10 @@ REGISTRY_PERSON_URL *registry_person_url_reallocate(REGISTRY_PERSON *p, REGISTRY
     tpu->usages = pu->usages;
     tpu->flags = pu->flags;
 
-    freez(pu);
     return tpu;
 }
 
 
-void registry_person_url_del(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) {
-    debug(D_REGISTRY, "Registry: registry_person_url_del('%s', '%s')", p->guid, pu->url->url);
-    registry_person_url_index_del(p, pu);
-}
-
 // ----------------------------------------------------------------------------
 // PERSON
 
@@ -157,13 +196,26 @@ REGISTRY_PERSON *registry_person_get(const char *person_guid, time_t when) {
     return p;
 }
 
+void registry_person_del(REGISTRY_PERSON *p) {
+    debug(D_REGISTRY, "Registry: registry_person_del('%s'): creating dictionary of urls", p->guid);
+
+    while(p->person_urls.root)
+        registry_person_unlink_from_url(p, (REGISTRY_PERSON_URL *)p->person_urls.root);
+
+    debug(D_REGISTRY, "Registry: deleting person '%s' from persons registry", p->guid);
+    dictionary_del(registry.persons, p->guid);
+
+    debug(D_REGISTRY, "Registry: freeing person '%s'", p->guid);
+    freez(p);
+}
+
 // ----------------------------------------------------------------------------
 // LINKING OF OBJECTS
 
 REGISTRY_PERSON_URL *registry_person_link_to_url(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when) {
     debug(D_REGISTRY, "registry_person_link_to_url('%s', '%s', '%s'): searching for URL in person", p->guid, m->guid, u->url);
 
-    REGISTRY_PERSON_URL *pu = registry_person_url_find(p, u->url);
+    REGISTRY_PERSON_URL *pu = registry_person_url_index_find(p, u->url);
     if(!pu) {
         debug(D_REGISTRY, "registry_person_link_to_url('%s', '%s', '%s'): not found", p->guid, m->guid, u->url);
         pu = registry_person_url_allocate(p, m, u, name, namelen, when);
@@ -207,3 +259,6 @@ REGISTRY_PERSON_URL *registry_person_link_to_url(REGISTRY_PERSON *p, REGISTRY_MA
     return pu;
 }
 
+void registry_person_unlink_from_url(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) {
+    registry_person_url_free(p, pu);
+}

diff --git a/src/registry_person.h b/src/registry_person.h
index 1355cc7296348f938c3424bb6cef4cdc03848353..2bbefa318ee57f09ee18d3ae1f97a5d4f9eed4c6 100644 (file)
--- a/src/registry_person.h
+++ b/src/registry_person.h
@@ -39,14 +39,22 @@ struct registry_person {
 };
 typedef struct registry_person REGISTRY_PERSON;
 
-extern REGISTRY_PERSON *registry_person_find(const char *person_guid);
+// PERSON_URL
+extern REGISTRY_PERSON_URL *registry_person_url_index_find(REGISTRY_PERSON *p, const char *url);
+extern REGISTRY_PERSON_URL *registry_person_url_index_add(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((warn_unused_result));
+extern REGISTRY_PERSON_URL *registry_person_url_index_del(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((warn_unused_result));
+
 extern REGISTRY_PERSON_URL *registry_person_url_allocate(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when);
 extern REGISTRY_PERSON_URL *registry_person_url_reallocate(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when, REGISTRY_PERSON_URL *pu);
+
+// PERSON
+extern REGISTRY_PERSON *registry_person_find(const char *person_guid);
 extern REGISTRY_PERSON *registry_person_allocate(const char *person_guid, time_t when);
 extern REGISTRY_PERSON *registry_person_get(const char *person_guid, time_t when);
-extern REGISTRY_PERSON_URL *registry_person_link_to_url(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when);
+extern void registry_person_del(REGISTRY_PERSON *p);
 
-extern REGISTRY_PERSON_URL *registry_person_url_find(REGISTRY_PERSON *p, const char *url);
-extern void registry_person_url_del(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu);
+// LINKING PERSON -> PERSON_URL
+extern REGISTRY_PERSON_URL *registry_person_link_to_url(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when);
+extern void registry_person_unlink_from_url(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu);
 
 #endif //NETDATA_REGISTRY_PERSON_H

diff --git a/src/registry_url.c b/src/registry_url.c
index 9132013f829c1904a3b69bae191c19d8fe3e620b..52d36a898e285b713bc7ac4e969d9118071e2313 100644 (file)
--- a/src/registry_url.c
+++ b/src/registry_url.c
@@ -9,8 +9,13 @@ int registry_url_compare(void *a, void *b) {
     else return strcmp(((REGISTRY_URL *)a)->url, ((REGISTRY_URL *)b)->url);
 }
 
-#define registry_url_index_add(rc) (REGISTRY_URL *)avl_insert(&(registry.registry_urls_root_index), (avl *)(rc))
-#define registry_url_index_del(rc) (REGISTRY_URL *)avl_remove(&(registry.registry_urls_root_index), (avl *)(rc))
+inline REGISTRY_URL *registry_url_index_add(REGISTRY_URL *u) {
+    return (REGISTRY_URL *)avl_insert(&(registry.registry_urls_root_index), (avl *)(u));
+}
+
+inline REGISTRY_URL *registry_url_index_del(REGISTRY_URL *u) {
+    return (REGISTRY_URL *)avl_remove(&(registry.registry_urls_root_index), (avl *)(u));
+}
 
 REGISTRY_URL *registry_url_get(const char *url, size_t urllen) {
     // protection from too big URLs
@@ -63,11 +68,17 @@ void registry_url_unlink(REGISTRY_URL *u) {
     if(!u->links) {
         debug(D_REGISTRY, "Registry: registry_url_unlink('%s'): No more links for this URL", u->url);
         REGISTRY_URL *n = registry_url_index_del(u);
-        if(n != u)
-            error("INTERNAL ERROR: registry_url_unlink(): request to delete url '%s', deleted url '%s'", u->url, n->url);
+        if(!n) {
+            error("INTERNAL ERROR: registry_url_unlink('%s'): cannot find url in index", u->url);
+        }
+        else {
+            if(n != u) {
+                error("INTERNAL ERROR: registry_url_unlink('%s'): deleted different url '%s'", u->url, n->url);
+            }
 
-        registry.urls_memory -= sizeof(REGISTRY_URL) + n->len; // no need for +1, 1 is already in REGISTRY_URL
-        freez(n);
+            registry.urls_memory -= sizeof(REGISTRY_URL) + n->len; // no need for +1, 1 is already in REGISTRY_URL
+            freez(n);
+        }
     }
     else
         debug(D_REGISTRY, "Registry: registry_url_unlink('%s'): URL has %u links left", u->url, u->links);

diff --git a/src/registry_url.h b/src/registry_url.h
index 1a13ca549c81f83b45f0d3b44f7dc06e08ceecf1..34c1d189669cd412a69fc5f19a1eb659a7be765a 100644 (file)
--- a/src/registry_url.h
+++ b/src/registry_url.h
@@ -20,7 +20,12 @@ struct registry_url {
 };
 typedef struct registry_url REGISTRY_URL;
 
+// REGISTRY_URL INDEX
 extern int registry_url_compare(void *a, void *b);
+extern REGISTRY_URL *registry_url_index_del(REGISTRY_URL *u);
+extern REGISTRY_URL *registry_url_index_add(REGISTRY_URL *u) __attribute__((returns_nonnull));
+
+// REGISTRY_URL MANAGEMENT
 extern REGISTRY_URL *registry_url_get(const char *url, size_t urllen) __attribute__((returns_nonnull));
 extern void registry_url_link(REGISTRY_URL *u);
 extern void registry_url_unlink(REGISTRY_URL *u);