BUG/MINOR: counters: fix unexpected 127 char GUID truncation for shm-stats-file objects

As reported by GH @phihos on GH #3320, using the shm-stats-file feature
with objects exceeding 127 chars would result in object name being
unexpectedly truncated, while GUID API supports up to 128 chars.

Indeed, with the config below, and shm-stats-file enabled:
   server s1  127.0.0.1:1 guid srv:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:SRV_1 disabled
    server s10 127.0.0.1:1 guid srv:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:SRV_10 disabled

haproxy would store the second server object with the same id as the first
one, but upon reload, only the first one would be restored, which would
eventually cause shm-stats-file slot exhaustion with repetitive reloads.

@phihos, found out the underlying issue, in counters.c we used snprintf()
with sizeof(shm_obj->guid) - 1 as <size> parameter, while we should have
use sizeof(shm_obj->guid) instead since shm_obj->guid already takes the
terminating NULL byte into account.

So we simply apply the fix suggested by @phihos, and hopefully this should
solve the shm-stats-file slot leak that was observed.

Unfortunately, for now, we cannot warn the user that a duplicate
shm-stats-file object was found, because we accept duplicate objects
by design for 2 reasons. The first one is for a new process to be able
to change the object type for a previously known GUID while allowing
previous processes to use the old object as long as they are alive.
The second reason is that upon startup we cannot afford to scan the
whole object list, as soon as we find a match (type + GUID), we bind
the object, and this way we avoid unnecessary lookup time.

Perhaps we have room for improvement in the future, but for now let's
keep it this way.

It should be backported to 3.3

Big thanks to @phihos for the bug description, analysis and
suggestions.

diff --git a/src/counters.c b/src/counters.c
index 3e19fec4581cac3593d594810428a777b3dcc421..7e75154ec4adbaa5124ddc707bd05264f86cd8ea 100644 (file)
--- a/src/counters.c
+++ b/src/counters.c
@@ -109,7 +109,7 @@ static int _counters_shared_prepare(struct counters_shared *shared,
 
                        shm_obj = shm_stats_file_add_object(errmsg);
                        if (shm_obj) {
-                               snprintf(shm_obj->guid, sizeof(shm_obj->guid)- 1, "%s", guid_get(guid));
+                               snprintf(shm_obj->guid, sizeof(shm_obj->guid), "%s", guid_get(guid));
                                if (is_be) {
                                        shm_obj->type = SHM_STATS_FILE_OBJECT_TYPE_BE;
                                        be_shared = (struct be_counters_shared *)shared;