[commit] master: streamline pretty-printing of message flags

[Oswald B. <os...@us...>]

commit a49017f481a7da71af1bcfdad8f9a517f88ac1e0
Author: Oswald Buddenhagen <os...@us...>
Date:   Wed Jun 15 17:17:23 2022 +0200

    streamline pretty-printing of message flags
    
    wrap make_flags() into fmt_flags() which returns a (struct-wrapped)
    string, so the calls can be inlined into the printf statements, without
    reserving buffers.
    
    we locally force optimization, so copy elision is always done, as debug
    builds would otherwise suffer a somewhat unreasonable performance hit.

 src/common.h     |  2 ++
 src/driver.c     | 11 ++++++++++-
 src/driver.h     |  3 ++-
 src/drv_proxy.c  | 23 ++++-------------------
 src/sync.c       | 13 +++++--------
 src/sync_state.c |  5 ++---
 6 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/src/common.h b/src/common.h
index f99b6475..6a356f4f 100644
--- a/src/common.h
+++ b/src/common.h
@@ -53,10 +53,12 @@ typedef unsigned long ulong;
 # define ATTR_UNUSED __attribute__((unused))
 # define ATTR_NORETURN __attribute__((noreturn))
 # define ATTR_PRINTFLIKE(fmt,var) __attribute__((format(printf,fmt,var)))
+# define ATTR_OPTIMIZE __attribute__((optimize("2")))
 #else
 # define ATTR_UNUSED
 # define ATTR_NORETURN
 # define ATTR_PRINTFLIKE(fmt,var)
+# define ATTR_OPTIMIZE
 #endif
 
 #if defined(__clang__)
diff --git a/src/driver.c b/src/driver.c
index ef637194..2df6948d 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -21,7 +21,7 @@ cleanup_drivers( void )
 // Keep the MESSAGE_FLAGS in sync (grep that)!
 const char MsgFlags[] = { 'D', 'F', 'P', 'R', 'S', 'T' };
 
-void
+static void
 make_flags( uchar flags, char *buf )
 {
 	uint i, d;
@@ -32,6 +32,15 @@ make_flags( uchar flags, char *buf )
 	buf[d] = 0;
 }
 
+flag_str_t
+fmt_flags( uchar flags )
+{
+	flag_str_t buf;
+
+	make_flags( flags, buf.str );
+	return buf;
+}
+
 uint
 count_generic_messages( message_t *msgs )
 {
diff --git a/src/driver.h b/src/driver.h
index e810bf1f..929e095a 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -46,7 +46,8 @@ BIT_ENUM(
 )
 
 extern const char MsgFlags[F__NUM_BITS];
-void make_flags( uchar flags, char *buf );
+typedef struct { char str[F__NUM_BITS + 1]; } flag_str_t;
+flag_str_t ATTR_OPTIMIZE /* force RVO */ fmt_flags( uchar flags );
 
 /* For message->status */
 BIT_ENUM(
diff --git a/src/drv_proxy.c b/src/drv_proxy.c
index 2dddb7fc..45d63528 100644
--- a/src/drv_proxy.c
+++ b/src/drv_proxy.c
@@ -198,7 +198,6 @@ proxy_do_@name@_cb( gen_cmd_t *gcmd )
 {
 	@name@_cmd_t *cmd = (@name@_cmd_t *)gcmd;
 
-	@pre_print_cb_args@
 	debug( "%s[% 2d] Callback enter @name@@print_fmt_cb_args@\n", cmd->ctx->label, cmd->tag@print_pass_cb_args@ );
 	@print_cb_args@
 	cmd->callback( @pass_cb_args@cmd->callback_aux );
@@ -257,10 +256,9 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 //# DEFINE load_box_print_pass_cb_args , cmd->sts, cmd->total_msgs, cmd->recent_msgs
 //# DEFINE load_box_print_cb_args
 	if (cmd->sts == DRV_OK) {
-		char fbuf[as(MsgFlags) + 1];
 		for (message_t *msg = cmd->msgs; msg; msg = msg->next)
 			debug( "  uid=%-5u flags=%-4s size=%-6u tuid=%." stringify(TUIDL) "s\n",
-			       msg->uid, (msg->status & M_FLAGS) ? (make_flags( msg->flags, fbuf ), fbuf) : "?", msg->size, *msg->tuid ? msg->tuid : "?" );
+			       msg->uid, (msg->status & M_FLAGS) ? fmt_flags( msg->flags ).str : "?", msg->size, *msg->tuid ? msg->tuid : "?" );
 	}
 //# END
 
@@ -281,12 +279,8 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 //# END
 //# DEFINE fetch_msg_print_fmt_args , uid=%u, want_flags=%s, want_date=%s
 //# DEFINE fetch_msg_print_pass_args , msg->uid, !(msg->status & M_FLAGS) ? "yes" : "no", data->date ? "yes" : "no"
-//# DEFINE fetch_msg_pre_print_cb_args
-	char fbuf[as(MsgFlags) + 1];
-	make_flags( cmd->data->flags, fbuf );
-//# END
 //# DEFINE fetch_msg_print_fmt_cb_args , flags=%s, date=%lld, size=%u
-//# DEFINE fetch_msg_print_pass_cb_args , fbuf, (long long)cmd->data->date, cmd->data->len
+//# DEFINE fetch_msg_print_pass_cb_args , fmt_flags( cmd->data->flags ).str, (long long)cmd->data->date, cmd->data->len
 //# DEFINE fetch_msg_print_cb_args
 	if (cmd->sts == DRV_OK && (DFlags & DEBUG_DRV_ALL)) {
 		printf( "%s=========\n", cmd->ctx->label );
@@ -296,12 +290,8 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 	}
 //# END
 
-//# DEFINE store_msg_pre_print_args
-	char fbuf[as(MsgFlags) + 1];
-	make_flags( data->flags, fbuf );
-//# END
 //# DEFINE store_msg_print_fmt_args , flags=%s, date=%lld, size=%u, to_trash=%s
-//# DEFINE store_msg_print_pass_args , fbuf, (long long)data->date, data->len, to_trash ? "yes" : "no"
+//# DEFINE store_msg_print_pass_args , fmt_flags( data->flags ).str, (long long)data->date, data->len, to_trash ? "yes" : "no"
 //# DEFINE store_msg_print_args
 	if (DFlags & DEBUG_DRV_ALL) {
 		printf( "%s>>>>>>>>>\n", ctx->label );
@@ -311,13 +301,8 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 	}
 //# END
 
-//# DEFINE set_msg_flags_pre_print_args
-	char fbuf1[as(MsgFlags) + 1], fbuf2[as(MsgFlags) + 1];
-	make_flags( add, fbuf1 );
-	make_flags( del, fbuf2 );
-//# END
 //# DEFINE set_msg_flags_print_fmt_args , uid=%u, add=%s, del=%s
-//# DEFINE set_msg_flags_print_pass_args , uid, fbuf1, fbuf2
+//# DEFINE set_msg_flags_print_pass_args , uid, fmt_flags( add ).str, fmt_flags( del ).str
 //# DEFINE set_msg_flags_checked sts == DRV_OK
 
 //# DEFINE trash_msg_print_fmt_args , uid=%u
diff --git a/src/sync.c b/src/sync.c
index 0b19e4d0..3d12aea4 100644
--- a/src/sync.c
+++ b/src/sync.c
@@ -71,9 +71,8 @@ sanitize_flags( uchar tflags, sync_vars_t *svars, int t )
 		// each mailbox can support different flags according to the IMAP spec.
 		uchar bflags = tflags & ~(svars->good_flags[t] | svars->bad_flags[t]);
 		if (bflags) {
-			char bfbuf[16];
-			make_flags( bflags, bfbuf );
-			notice( "Notice: %s store does not support flag(s) '%s'; not propagating.\n", str_fn[t], bfbuf );
+			notice( "Notice: %s store does not support flag(s) '%s'; not propagating.\n",
+			        str_fn[t], fmt_flags( bflags ).str );
 			svars->bad_flags[t] |= bflags;
 		}
 	}
@@ -1014,11 +1013,9 @@ box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux
 						}
 						srec->aflags[t] = sflags & ~srec->flags;
 						srec->dflags[t] = ~sflags & srec->flags;
-						if ((DFlags & DEBUG_SYNC) && (srec->aflags[t] || srec->dflags[t])) {
-							char afbuf[16], dfbuf[16]; /* enlarge when support for keywords is added */
-							make_flags( srec->aflags[t], afbuf );
-							make_flags( srec->dflags[t], dfbuf );
-							debug( "  %sing flags: +%s -%s\n", str_hl[t], afbuf, dfbuf );
+						if (srec->aflags[t] || srec->dflags[t]) {
+							debug( "  %sing flags: +%s -%s\n", str_hl[t],
+							       fmt_flags( srec->aflags[t] ).str, fmt_flags( srec->dflags[t] ).str );
 						}
 					}
 				}
diff --git a/src/sync_state.c b/src/sync_state.c
index 5c7f1146..b35054bc 100644
--- a/src/sync_state.c
+++ b/src/sync_state.c
@@ -467,11 +467,10 @@ save_state( sync_vars_t *svars )
 	for (sync_rec_t *srec = svars->srecs; srec; srec = srec->next) {
 		if (srec->status & S_DEAD)
 			continue;
-		char fbuf[16];  // enlarge when support for keywords is added
-		make_flags( srec->flags, fbuf );
 		Fprintf( svars->nfp, "%u %u %s%s%s\n", srec->uid[F], srec->uid[N],
 		         (srec->status & S_DUMMY(F)) ? "<" : (srec->status & S_DUMMY(N)) ? ">" : "",
-		         (srec->status & S_SKIPPED) ? "^" : (srec->status & S_EXPIRED) ? "~" : "", fbuf );
+		         (srec->status & S_SKIPPED) ? "^" : (srec->status & S_EXPIRED) ? "~" : "",
+		         fmt_flags( srec->flags ).str );
 	}
 
 	Fclose( svars->nfp, 1 );