From a9dc5564aa1bc44d882fc013ed06bc7151480b6b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:18:25 +0200 Subject: [PATCH 01/14] Only store hostnames of domains and upstream servers if they have changed. Currently, we are storing their host names each time we reresolve the IP addresses, even if they have been found to be identical. This allows us to save memory in FTL processes that are running for a long time without being restarted. Signed-off-by: DL6ER --- resolve.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/resolve.c b/resolve.c index 3bd7b0c39..3db358645 100644 --- a/resolve.c +++ b/resolve.c @@ -88,6 +88,7 @@ void resolveClients(bool onlynew) // Lock data when obtaining IP of this client lock_shm(); const char* ipaddr = getstr(clients[clientID].ippos); + const char* name = getstr(clients[clientID].namepos); unlock_shm(); // Important: Don't hold a lock while resolving as the main thread @@ -96,7 +97,13 @@ void resolveClients(bool onlynew) // Finally, lock data when storing obtained hostname lock_shm(); - clients[clientID].namepos = addstr(hostname); + // Only store new hostname if it changed + if(hostname != NULL && strcmp(name, hostname) != 0) + { + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 + clients[clientID].namepos = addstr(hostname); + } clients[clientID].new = false; unlock_shm(); } @@ -119,6 +126,7 @@ void resolveForwardDestinations(bool onlynew) // Lock data when obtaining IP of this forward destination lock_shm(); const char* ipaddr = getstr(forwarded[forwardID].ippos); + const char* name = getstr(forwarded[forwardID].namepos); unlock_shm(); @@ -128,7 +136,13 @@ void resolveForwardDestinations(bool onlynew) // Finally, lock data when storing obtained hostname lock_shm(); - forwarded[forwardID].namepos = addstr(hostname); + // Only store new hostname if it changed + if(hostname != NULL && strcmp(name, hostname) != 0) + { + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 + forwarded[forwardID].namepos = addstr(hostname); + } forwarded[forwardID].new = false; unlock_shm(); } From 4184141557d8f6e42b5af012368f04de76476db9 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:28:54 +0200 Subject: [PATCH 02/14] Add debug output Signed-off-by: DL6ER --- resolve.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/resolve.c b/resolve.c index 3db358645..cdfbf24bb 100644 --- a/resolve.c +++ b/resolve.c @@ -104,6 +104,12 @@ void resolveClients(bool onlynew) // always initialized with an empty string at position 0 clients[clientID].namepos = addstr(hostname); } + else if(config.debug & DEBUG_SHMEM) + { + // Debug output + logg("Not adding \"%s\" (unchanged)", name); + } + // Mark entry as not new clients[clientID].new = false; unlock_shm(); } @@ -143,6 +149,12 @@ void resolveForwardDestinations(bool onlynew) // always initialized with an empty string at position 0 forwarded[forwardID].namepos = addstr(hostname); } + else if(config.debug & DEBUG_SHMEM) + { + // Debug output + logg("Not adding \"%s\" (unchanged)", name); + } + // Mark entry as not new forwarded[forwardID].new = false; unlock_shm(); } From 540dba6a53e2c85e742eb730472633f51c6dd989 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:31:38 +0200 Subject: [PATCH 03/14] Make sure we release allocated memory once we don't need it any longer Signed-off-by: DL6ER --- resolve.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/resolve.c b/resolve.c index cdfbf24bb..46b73d323 100644 --- a/resolve.c +++ b/resolve.c @@ -19,7 +19,7 @@ // once every hour #define RERESOLVE_INTERVAL 3600 -static const char *resolveHostname(const char *addr) +static char *resolveHostname(const char *addr) { // Get host name struct hostent *he = NULL; @@ -93,10 +93,11 @@ void resolveClients(bool onlynew) // Important: Don't hold a lock while resolving as the main thread // (dnsmasq) needs to be operable during the call to resolveHostname() - const char* hostname = resolveHostname(ipaddr); + char* hostname = resolveHostname(ipaddr); // Finally, lock data when storing obtained hostname lock_shm(); + // Only store new hostname if it changed if(hostname != NULL && strcmp(name, hostname) != 0) { @@ -106,11 +107,17 @@ void resolveClients(bool onlynew) } else if(config.debug & DEBUG_SHMEM) { - // Debug output + // Debugging output logg("Not adding \"%s\" (unchanged)", name); } + + // Release allocated memory + free(hostname); + // Mark entry as not new clients[clientID].new = false; + + // Unlock shared memory unlock_shm(); } } @@ -138,7 +145,7 @@ void resolveForwardDestinations(bool onlynew) // Important: Don't hold a lock while resolving as the main thread // (dnsmasq) needs to be operable during the call to resolveHostname() - const char* hostname = resolveHostname(ipaddr); + char* hostname = resolveHostname(ipaddr); // Finally, lock data when storing obtained hostname lock_shm(); @@ -151,11 +158,17 @@ void resolveForwardDestinations(bool onlynew) } else if(config.debug & DEBUG_SHMEM) { - // Debug output + // Debugging output logg("Not adding \"%s\" (unchanged)", name); } + + // Release allocated memory + free(hostname); + // Mark entry as not new forwarded[forwardID].new = false; + + // Unlock shared memory unlock_shm(); } } From e24360517f16c75b07be8979f0c6ebfd8906d623 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:35:33 +0200 Subject: [PATCH 04/14] Align debugging output to the output produced by addstr() Signed-off-by: DL6ER --- resolve.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resolve.c b/resolve.c index 46b73d323..7ff194d49 100644 --- a/resolve.c +++ b/resolve.c @@ -108,7 +108,7 @@ void resolveClients(bool onlynew) else if(config.debug & DEBUG_SHMEM) { // Debugging output - logg("Not adding \"%s\" (unchanged)", name); + logg("Not adding \"%s\" to buffer (unchanged)", name); } // Release allocated memory @@ -159,7 +159,7 @@ void resolveForwardDestinations(bool onlynew) else if(config.debug & DEBUG_SHMEM) { // Debugging output - logg("Not adding \"%s\" (unchanged)", name); + logg("Not adding \"%s\" to buffer (unchanged)", name); } // Release allocated memory From cc0b3782f14067aabe719edff0d871a09fe25369 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 10 Apr 2019 16:49:16 +0200 Subject: [PATCH 05/14] Review comments 1 Signed-off-by: DL6ER --- resolve.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/resolve.c b/resolve.c index 7ff194d49..d45a1d1dc 100644 --- a/resolve.c +++ b/resolve.c @@ -99,10 +99,10 @@ void resolveClients(bool onlynew) lock_shm(); // Only store new hostname if it changed + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 if(hostname != NULL && strcmp(name, hostname) != 0) { - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 clients[clientID].namepos = addstr(hostname); } else if(config.debug & DEBUG_SHMEM) @@ -112,7 +112,8 @@ void resolveClients(bool onlynew) } // Release allocated memory - free(hostname); + if(hostname != NULL) + free(hostname); // Mark entry as not new clients[clientID].new = false; @@ -150,10 +151,10 @@ void resolveForwardDestinations(bool onlynew) // Finally, lock data when storing obtained hostname lock_shm(); // Only store new hostname if it changed + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 if(hostname != NULL && strcmp(name, hostname) != 0) { - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 forwarded[forwardID].namepos = addstr(hostname); } else if(config.debug & DEBUG_SHMEM) @@ -163,7 +164,8 @@ void resolveForwardDestinations(bool onlynew) } // Release allocated memory - free(hostname); + if(hostname != NULL) + free(hostname); // Mark entry as not new forwarded[forwardID].new = false; From 8e67784b81b4cb3817917bfcee3e2aeaa1276d45 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 12 Apr 2019 16:17:34 +0200 Subject: [PATCH 06/14] Swap out common code in resolve.c into its own subroutine Signed-off-by: DL6ER --- resolve.c | 105 ++++++++++++++++++++++++------------------------------ 1 file changed, 47 insertions(+), 58 deletions(-) diff --git a/resolve.c b/resolve.c index d45a1d1dc..ad238d1c0 100644 --- a/resolve.c +++ b/resolve.c @@ -71,6 +71,37 @@ static char *resolveHostname(const char *addr) return hostname; } +// Resolve upstream destination host names +static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) +{ + // Get IP and host name strings + const char* ipaddr = getstr(ippos); + const char* oldname = getstr(oldnamepos); + + // Important: Don't hold a lock while resolving as the main thread + // (dnsmasq) needs to be operable during the call to resolveHostname() + char* newname = resolveHostname(ipaddr); + + // Only store new newname if it changed + if(newname != NULL && strcmp(oldname, newname) != 0) + { + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 + size_t newnamepos = addstr(newname); + if(newname != NULL) + free(newname); + return newnamepos; + } + else if(config.debug & DEBUG_SHMEM) + { + // Debugging output + logg("Not adding \"%s\" to buffer (unchanged)", oldname); + } + + // Not changed, return old namepos + return oldnamepos; +} + // Resolve client host names void resolveClients(bool onlynew) { @@ -85,41 +116,20 @@ void resolveClients(bool onlynew) if(onlynew && !clients[clientID].new) continue; - // Lock data when obtaining IP of this client - lock_shm(); - const char* ipaddr = getstr(clients[clientID].ippos); - const char* name = getstr(clients[clientID].namepos); - unlock_shm(); - - // Important: Don't hold a lock while resolving as the main thread - // (dnsmasq) needs to be operable during the call to resolveHostname() - char* hostname = resolveHostname(ipaddr); - - // Finally, lock data when storing obtained hostname - lock_shm(); + // Obtain/update hostname of this client + size_t oldnamepos = clients[clientID].namepos; + size_t newnamepos = resolveAndAddHostname(clients[clientID].ippos, oldnamepos); - // Only store new hostname if it changed - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 - if(hostname != NULL && strcmp(name, hostname) != 0) - { - clients[clientID].namepos = addstr(hostname); - } - else if(config.debug & DEBUG_SHMEM) + if(newnamepos != oldnamepos) { - // Debugging output - logg("Not adding \"%s\" to buffer (unchanged)", name); + // Need lock when storing obtained hostname + lock_shm(); + clients[clientID].namepos = newnamepos; + unlock_shm(); } - // Release allocated memory - if(hostname != NULL) - free(hostname); - // Mark entry as not new clients[clientID].new = false; - - // Unlock shared memory - unlock_shm(); } } @@ -137,41 +147,20 @@ void resolveForwardDestinations(bool onlynew) if(onlynew && !forwarded[forwardID].new) continue; - // Lock data when obtaining IP of this forward destination - lock_shm(); - const char* ipaddr = getstr(forwarded[forwardID].ippos); - const char* name = getstr(forwarded[forwardID].namepos); - unlock_shm(); - + // Obtain/update hostname of this client + size_t oldnamepos = forwarded[forwardID].namepos; + size_t newnamepos = resolveAndAddHostname(forwarded[forwardID].ippos, oldnamepos); - // Important: Don't hold a lock while resolving as the main thread - // (dnsmasq) needs to be operable during the call to resolveHostname() - char* hostname = resolveHostname(ipaddr); - - // Finally, lock data when storing obtained hostname - lock_shm(); - // Only store new hostname if it changed - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 - if(hostname != NULL && strcmp(name, hostname) != 0) + if(newnamepos != oldnamepos) { - forwarded[forwardID].namepos = addstr(hostname); + // Need lock when storing obtained hostname + lock_shm(); + forwarded[forwardID].namepos = newnamepos; + unlock_shm(); } - else if(config.debug & DEBUG_SHMEM) - { - // Debugging output - logg("Not adding \"%s\" to buffer (unchanged)", name); - } - - // Release allocated memory - if(hostname != NULL) - free(hostname); // Mark entry as not new forwarded[forwardID].new = false; - - // Unlock shared memory - unlock_shm(); } } From 9c4c9d701f0ed8b2eab7c07d207ed8f34d82bcd7 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 12 Apr 2019 16:23:50 +0200 Subject: [PATCH 07/14] Move global constants RESOLVE_INTERVAL and RERESOLVE_INTERVAL to FTL.h Signed-off-by: DL6ER --- FTL.h | 8 ++++++++ resolve.c | 8 -------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/FTL.h b/FTL.h index a4bba0955..b18408797 100644 --- a/FTL.h +++ b/FTL.h @@ -80,6 +80,14 @@ // can be 24 hours + 59 minutes #define OVERTIME_SLOTS ((MAXLOGAGE+1)*3600/OVERTIME_INTERVAL) +// Interval for resolving NEW client and upstream server host names [seconds] +// Default: 60 (once every minute) +#define RESOLVE_INTERVAL 60 + +// Interval for re-resolving ALL known host names [seconds] +// Default: 3600 (once every hour) +#define RERESOLVE_INTERVAL 3600 + // FTLDNS enums enum { DATABASE_WRITE_TIMER, EXIT_TIMER, GC_TIMER, LISTS_TIMER, REGEX_TIMER, ARP_TIMER, LAST_TIMER }; enum { QUERIES, FORWARDED, CLIENTS, DOMAINS, OVERTIME, WILDCARD }; diff --git a/resolve.c b/resolve.c index ad238d1c0..1191c676c 100644 --- a/resolve.c +++ b/resolve.c @@ -11,14 +11,6 @@ #include "FTL.h" #include "shmem.h" -// Resolve new client and upstream server host names -// once every minute -#define RESOLVE_INTERVAL 60 - -// Re-resolve client names -// once every hour -#define RERESOLVE_INTERVAL 3600 - static char *resolveHostname(const char *addr) { // Get host name From 3ab4076738fed9b8888e93a7165b7eb02fdacb39 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 14:14:32 +0200 Subject: [PATCH 08/14] Lock SHM when using getstr() and addstr() Signed-off-by: DL6ER --- resolve.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/resolve.c b/resolve.c index 1191c676c..219dd114d 100644 --- a/resolve.c +++ b/resolve.c @@ -67,8 +67,10 @@ static char *resolveHostname(const char *addr) static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) { // Get IP and host name strings + lock_shm(); const char* ipaddr = getstr(ippos); const char* oldname = getstr(oldnamepos); + unlock_shm(); // Important: Don't hold a lock while resolving as the main thread // (dnsmasq) needs to be operable during the call to resolveHostname() @@ -79,7 +81,9 @@ static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) { // We do not need to check for name == NULL as name is // always initialized with an empty string at position 0 + lock_shm(); size_t newnamepos = addstr(newname); + unlock_shm(); if(newname != NULL) free(newname); return newnamepos; From 89957271ea6c45542b548b03d30de0645f4b8abd Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 20:56:24 +0200 Subject: [PATCH 09/14] Newname can be freed without if as we check for NULL already one level higher Signed-off-by: DL6ER --- resolve.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/resolve.c b/resolve.c index 219dd114d..3052b663d 100644 --- a/resolve.c +++ b/resolve.c @@ -76,16 +76,17 @@ static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) // (dnsmasq) needs to be operable during the call to resolveHostname() char* newname = resolveHostname(ipaddr); - // Only store new newname if it changed + // Only store new newname if it is valid and differs from oldname + // We do not need to check for oldname == NULL as names are + // always initialized with an empty string at position 0 if(newname != NULL && strcmp(oldname, newname) != 0) { - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 lock_shm(); size_t newnamepos = addstr(newname); + // newname has already been checked against NULL + // so we can safely free it + free(newname); unlock_shm(); - if(newname != NULL) - free(newname); return newnamepos; } else if(config.debug & DEBUG_SHMEM) From cc88ed765afd6f260ff4a4c0371d8afc252acbbc Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:00:52 +0200 Subject: [PATCH 10/14] Lock more than less Signed-off-by: DL6ER --- resolve.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/resolve.c b/resolve.c index 3052b663d..33a998618 100644 --- a/resolve.c +++ b/resolve.c @@ -126,7 +126,9 @@ void resolveClients(bool onlynew) } // Mark entry as not new + lock_shm(); clients[clientID].new = false; + unlock_shm(); } } @@ -157,7 +159,9 @@ void resolveForwardDestinations(bool onlynew) } // Mark entry as not new + lock_shm(); forwarded[forwardID].new = false; + unlock_shm(); } } From 9c0da2f2091bc5edb00456d6fcca8f9370cba271 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:10:24 +0200 Subject: [PATCH 11/14] Wrap all access to the forwarded and clients structures in locks. This is sometimes a bit cumbersome, however, it is necessary as we cannot completely lock the routines because this would prevent DNS resolution from being able to work Signed-off-by: DL6ER --- resolve.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/resolve.c b/resolve.c index 33a998618..251c1263a 100644 --- a/resolve.c +++ b/resolve.c @@ -110,12 +110,18 @@ void resolveClients(bool onlynew) // If onlynew flag is set, we will only resolve new clients // If not, we will try to re-resolve all known clients - if(onlynew && !clients[clientID].new) + lock_shm(); + bool newlock = clients[clientID].new; + unlock_shm(); + if(onlynew && !newlock) continue; // Obtain/update hostname of this client + lock_shm(); size_t oldnamepos = clients[clientID].namepos; - size_t newnamepos = resolveAndAddHostname(clients[clientID].ippos, oldnamepos); + size_t ippos = clients[clientID].ippos; + unlock_shm(); + size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) { @@ -143,12 +149,18 @@ void resolveForwardDestinations(bool onlynew) // If onlynew flag is set, we will only resolve new upstream destinations // If not, we will try to re-resolve all known upstream destinations - if(onlynew && !forwarded[forwardID].new) + lock_shm(); + bool newflag = forwarded[forwardID].new; + unlock_shm(); + if(onlynew && !newflag) continue; // Obtain/update hostname of this client + lock_shm(); size_t oldnamepos = forwarded[forwardID].namepos; - size_t newnamepos = resolveAndAddHostname(forwarded[forwardID].ippos, oldnamepos); + size_t ippos = forwarded[forwardID].ippos; + unlock_shm(); + size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) { From b29b9abb49506a06dd7c5d866240d187f93dddbd Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:19:03 +0200 Subject: [PATCH 12/14] Also lock access to the counters and combine locks where convenient Signed-off-by: DL6ER --- resolve.c | 60 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/resolve.c b/resolve.c index 251c1263a..80ad74f2c 100644 --- a/resolve.c +++ b/resolve.c @@ -102,25 +102,28 @@ static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) // Resolve client host names void resolveClients(bool onlynew) { - int clientID; - for(clientID = 0; clientID < counters->clients; clientID++) + // Lock counter access here, we use a copy in the following loop + lock_shm(); + int clientscount = counters->clients; + unlock_shm(); + for(int clientID = 0; clientID < clientscount; clientID++) { // Memory validation validate_access("clients", clientID, true, __LINE__, __FUNCTION__, __FILE__); - // If onlynew flag is set, we will only resolve new clients - // If not, we will try to re-resolve all known clients + // Memory access needs to get locked lock_shm(); bool newlock = clients[clientID].new; + size_t ippos = clients[clientID].ippos; + size_t oldnamepos = clients[clientID].namepos; unlock_shm(); + + // If onlynew flag is set, we will only resolve new clients + // If not, we will try to re-resolve all known clients if(onlynew && !newlock) continue; // Obtain/update hostname of this client - lock_shm(); - size_t oldnamepos = clients[clientID].namepos; - size_t ippos = clients[clientID].ippos; - unlock_shm(); size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) @@ -128,38 +131,45 @@ void resolveClients(bool onlynew) // Need lock when storing obtained hostname lock_shm(); clients[clientID].namepos = newnamepos; + clients[clientID].new = false; unlock_shm(); } - // Mark entry as not new - lock_shm(); - clients[clientID].new = false; - unlock_shm(); + // Mark entry as not new even when we don't have a new host name + if(clients[clientID].new) + { + lock_shm(); + clients[clientID].new = false; + unlock_shm(); + } } } // Resolve upstream destination host names void resolveForwardDestinations(bool onlynew) { - int forwardID; - for(forwardID = 0; forwardID < counters->forwarded; forwardID++) + // Lock counter access here, we use a copy in the following loop + lock_shm(); + int forwardedcount = counters->forwarded; + unlock_shm(); + for(int forwardID = 0; forwardID < forwardedcount; forwardID++) { // Memory validation validate_access("forwarded", forwardID, true, __LINE__, __FUNCTION__, __FILE__); - // If onlynew flag is set, we will only resolve new upstream destinations - // If not, we will try to re-resolve all known upstream destinations + // Memory access needs to get locked lock_shm(); bool newflag = forwarded[forwardID].new; + size_t ippos = forwarded[forwardID].ippos; + size_t oldnamepos = forwarded[forwardID].namepos; unlock_shm(); + + // If onlynew flag is set, we will only resolve new upstream destinations + // If not, we will try to re-resolve all known upstream destinations if(onlynew && !newflag) continue; // Obtain/update hostname of this client - lock_shm(); - size_t oldnamepos = forwarded[forwardID].namepos; - size_t ippos = forwarded[forwardID].ippos; - unlock_shm(); size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) @@ -167,13 +177,17 @@ void resolveForwardDestinations(bool onlynew) // Need lock when storing obtained hostname lock_shm(); forwarded[forwardID].namepos = newnamepos; + forwarded[forwardID].new = false; unlock_shm(); } // Mark entry as not new - lock_shm(); - forwarded[forwardID].new = false; - unlock_shm(); + if(forwarded[forwardID].new) + { + lock_shm(); + forwarded[forwardID].new = false; + unlock_shm(); + } } } From cfa9120ccc9ec3ed821d0c9e44382f92e963ac22 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:22:24 +0200 Subject: [PATCH 13/14] Use newflag instead of .new booleans to avoid needing a further lock Signed-off-by: DL6ER --- resolve.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/resolve.c b/resolve.c index 80ad74f2c..2754260de 100644 --- a/resolve.c +++ b/resolve.c @@ -113,14 +113,14 @@ void resolveClients(bool onlynew) // Memory access needs to get locked lock_shm(); - bool newlock = clients[clientID].new; + bool newflag = clients[clientID].new; size_t ippos = clients[clientID].ippos; size_t oldnamepos = clients[clientID].namepos; unlock_shm(); // If onlynew flag is set, we will only resolve new clients // If not, we will try to re-resolve all known clients - if(onlynew && !newlock) + if(onlynew && !newflag) continue; // Obtain/update hostname of this client @@ -132,11 +132,12 @@ void resolveClients(bool onlynew) lock_shm(); clients[clientID].namepos = newnamepos; clients[clientID].new = false; + newflag = false; unlock_shm(); } // Mark entry as not new even when we don't have a new host name - if(clients[clientID].new) + if(newflag) { lock_shm(); clients[clientID].new = false; @@ -178,11 +179,12 @@ void resolveForwardDestinations(bool onlynew) lock_shm(); forwarded[forwardID].namepos = newnamepos; forwarded[forwardID].new = false; + newflag = false; unlock_shm(); } // Mark entry as not new - if(forwarded[forwardID].new) + if(newflag) { lock_shm(); forwarded[forwardID].new = false; From 7a5eb9d715fdf2e793dddf4b5355dd727d9d5f3e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:35:57 +0200 Subject: [PATCH 14/14] Simplify code further. This removes the if statements at the end of the client/forward loops Signed-off-by: DL6ER --- resolve.c | 43 +++++++++++-------------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/resolve.c b/resolve.c index 2754260de..5cfdab1bd 100644 --- a/resolve.c +++ b/resolve.c @@ -126,23 +126,13 @@ void resolveClients(bool onlynew) // Obtain/update hostname of this client size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); - if(newnamepos != oldnamepos) - { - // Need lock when storing obtained hostname - lock_shm(); - clients[clientID].namepos = newnamepos; - clients[clientID].new = false; - newflag = false; - unlock_shm(); - } + lock_shm(); + // Store obtained host name (may be unchanged) + clients[clientID].namepos = newnamepos; - // Mark entry as not new even when we don't have a new host name - if(newflag) - { - lock_shm(); - clients[clientID].new = false; - unlock_shm(); - } + // Mark entry as not new + clients[clientID].new = false; + unlock_shm(); } } @@ -173,23 +163,12 @@ void resolveForwardDestinations(bool onlynew) // Obtain/update hostname of this client size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); - if(newnamepos != oldnamepos) - { - // Need lock when storing obtained hostname - lock_shm(); - forwarded[forwardID].namepos = newnamepos; - forwarded[forwardID].new = false; - newflag = false; - unlock_shm(); - } - + lock_shm(); + // Store obtained host name (may be unchanged) + forwarded[forwardID].namepos = newnamepos; // Mark entry as not new - if(newflag) - { - lock_shm(); - forwarded[forwardID].new = false; - unlock_shm(); - } + forwarded[forwardID].new = false; + unlock_shm(); } }