[PATCH] Streamline messaging and serverid handling

Ralph Boehme rb at sernet.de
Mon Oct 19 08:41:39 UTC 2015


Hi!

On Sat, Oct 17, 2015 at 03:37:48PM +0200, Volker Lendecke wrote:
> Attached find a patchset that moves generation of our unique
> id down into messaging_dgm.c, removing the global
> my_unique_id on its way.
> 
> This way every process gets a real unique id, not just the
> ones that take special care.

nice!

> Review&push appreciated!

Some minor nitpicking:

> +	do {
> +		generate_random_buffer((uint8_t *)&unique, sizeof(unique));
> +	} while (unique == 0xFFFFFFFFFFFFFFFFULL); /* SERVERID_UNIQUE_ID_NOT_TO_VERIFY */
> +

the C99 standard has macros for minimum-width integer constants, we
may want to use them:

do {
	generate_random_buffer((uint8_t *)&unique, sizeof(unique));
} while (unique == UINT64_C(0xFFFFFFFFFFFFFFFF)); /* SERVERID_UNIQUE_ID_NOT_TO_VERIFY */

>  	unique_len = snprintf(buf, sizeof(buf), "%ju\n", (uintmax_t)unique);

Casting fixed typed integers via other types that seems compatible in
2015 just feels wrong. Escpecially since C99 has macros for this
purpose.

SowWhile at it, I've prepared a patch on top of your patchset that
uses the C99 format specifier macros for reading and writing
unique. What do you think?

The following commits are missing your signed-off:

lib: Remove unused procid_self()
lib: Remove unused global my_unique_id

Other then that, all reviewed-by: me.

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From 934466ad2af595b10cefd8266ef14b314ed5e0aa Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 19 Oct 2015 10:34:14 +0200
Subject: [PATCH] lib:messaging: use format specifier macros for unique id

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/lib/messages_dgm.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index af31ffc..65abd1a 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -119,7 +119,7 @@ static int messaging_dgm_lockfile_create(struct messaging_dgm_context *ctx,
 		generate_random_buffer((uint8_t *)&unique, sizeof(unique));
 	} while (unique == 0xFFFFFFFFFFFFFFFFULL); /* SERVERID_UNIQUE_ID_NOT_TO_VERIFY */
 
-	unique_len = snprintf(buf, sizeof(buf), "%ju\n", (uintmax_t)unique);
+	unique_len = snprintf(buf, sizeof(buf), "%" PRIu64 "\n", unique);
 
 	/* shorten a potentially preexisting file */
 
@@ -323,8 +323,8 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
 {
 	char buf[25];
 	ssize_t rw_ret;
-	unsigned long long unique;
-	char *endptr;
+	uint64_t unique;
+	int ret;
 
 	rw_ret = pread(fd, buf, sizeof(buf)-1, 0);
 	if (rw_ret == -1) {
@@ -332,14 +332,8 @@ static int messaging_dgm_read_unique(int fd, uint64_t *punique)
 	}
 	buf[rw_ret] = '\0';
 
-	unique = strtoull(buf, &endptr, 10);
-	if ((unique == 0) && (errno == EINVAL)) {
-		return EINVAL;
-	}
-	if ((unique == ULLONG_MAX) && (errno == ERANGE)) {
-		return ERANGE;
-	}
-	if (endptr[0] != '\n') {
+	ret = sscanf(buf, SCNu64, &unique);
+	if ((ret != 1) || (unique == 0)) {
 		return EINVAL;
 	}
 	*punique = unique;
-- 
2.1.0



More information about the samba-technical mailing list