base36enc: abuse C99 compound literals lifetime.

C99 introduced compound literals (`(char[14]){0}` - literal of array).
Compound literals have same lifetime as block local variable, ie till
the end of block. There for it is save to initiate several of them in
same block and assume they are all live.

This way we may rewrite base36enc into macros which uses compound
literal instead of static variable to extend its result lifetime.

Author: funny-falcon

src/backup.c

@@ -735,7 +735,7 @@ do_backup(InstanceState *instanceState, pgSetBackupParams *set_backup_params,
 			/* don't care about freeing base36enc_dup memory, we exit anyway */
 			elog(ERROR, "Can't assign backup_id from requested start_time (%s), "
 						"this time must be later that backup %s",
-				base36enc_dup(start_time), base36enc_dup(latest_backup_id));
+				base36enc(start_time), base36enc(latest_backup_id));
 
 		current.backup_id = start_time;
 		pgBackupInitDir(&current, instanceState->instance_backup_subdir_path);

src/catalog.c

@@ -1219,7 +1219,6 @@ catalog_get_last_data_backup(parray *backup_list, TimeLineID tli, time_t current
 	int			i;
 	pgBackup   *full_backup = NULL;
 	pgBackup   *tmp_backup = NULL;
-	char 	   *invalid_backup_id;
 
 	/* backup_list is sorted in order of descending ID */
 	for (i = 0; i < parray_num(backup_list); i++)
@@ -1255,20 +1254,14 @@ catalog_get_last_data_backup(parray *backup_list, TimeLineID tli, time_t current
 			{
 				/* broken chain */
 				case ChainIsBroken:
-					invalid_backup_id = base36enc_dup(tmp_backup->parent_backup);
-
 					elog(WARNING, "Backup %s has missing parent: %s. Cannot be a parent",
-						base36enc(backup->start_time), invalid_backup_id);
-					pg_free(invalid_backup_id);
+						base36enc(backup->start_time), base36enc(tmp_backup->parent_backup));
 					continue;
 
 				/* chain is intact, but at least one parent is invalid */
 				case ChainIsInvalid:
-					invalid_backup_id = base36enc_dup(tmp_backup->start_time);
-
 					elog(WARNING, "Backup %s has invalid parent: %s. Cannot be a parent",
-						base36enc(backup->start_time), invalid_backup_id);
-					pg_free(invalid_backup_id);
+						base36enc(backup->start_time), base36enc(tmp_backup->start_time));
 					continue;
 
 				/* chain is ok */

src/delete.c

@@ -451,7 +451,6 @@ do_retention_merge(InstanceState *instanceState, parray *backup_list,
 	/* Merging happens here */
 	for (i = 0; i < parray_num(to_keep_list); i++)
 	{
-		char		*keep_backup_id = NULL;
 		pgBackup	*full_backup = NULL;
 		parray	    *merge_list = NULL;
 
@@ -488,10 +487,9 @@ do_retention_merge(InstanceState *instanceState, parray *backup_list,
 		 * backups from purge_list.
 		 */
 
-		keep_backup_id = base36enc_dup(keep_backup->start_time);
 		elog(INFO, "Merge incremental chain between full backup %s and backup %s",
-					base36enc(full_backup->start_time), keep_backup_id);
-		pg_free(keep_backup_id);
+					base36enc(full_backup->start_time),
+					base36enc(keep_backup->start_time));
 
 		merge_list = parray_new();
 
@@ -599,8 +597,6 @@ do_retention_purge(parray *to_keep_list, parray *to_purge_list)
 		 */
 		for (i = 0; i < parray_num(to_keep_list); i++)
 		{
-			char		*keeped_backup_id;
-
 			pgBackup   *keep_backup = (pgBackup *) parray_get(to_keep_list, i);
 
 			/* item could have been nullified in merge */
@@ -611,24 +607,22 @@ do_retention_purge(parray *to_keep_list, parray *to_purge_list)
 			if (keep_backup->backup_mode == BACKUP_MODE_FULL)
 				continue;
 
-			keeped_backup_id = base36enc_dup(keep_backup->start_time);
-
 			elog(LOG, "Check if backup %s is parent of backup %s",
-						base36enc(delete_backup->start_time), keeped_backup_id);
+						base36enc(delete_backup->start_time),
+						base36enc(keep_backup->start_time));
 
 			if (is_parent(delete_backup->start_time, keep_backup, true))
 			{
 
 				/* We must not delete this backup, evict it from purge list */
 				elog(LOG, "Retain backup %s because his "
 					"descendant %s is guarded by retention",
-						base36enc(delete_backup->start_time), keeped_backup_id);
+						base36enc(delete_backup->start_time),
+						base36enc(keep_backup->start_time));
 
 				purge = false;
-				pg_free(keeped_backup_id);
 				break;
 			}
-			pg_free(keeped_backup_id);
 		}
 
 		/* Retain backup */

src/merge.c

@@ -223,11 +223,9 @@ do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool
 			}
 			if (!dest_backup)
 			{
-				char *tmp_backup_id = base36enc_dup(full_backup->start_time);
 				elog(ERROR, "Full backup %s has unfinished merge with missing backup %s",
-								tmp_backup_id,
+								base36enc(full_backup->start_time),
 								base36enc(full_backup->merge_dest_backup));
-				pg_free(tmp_backup_id);
 			}
 		}
 		else if (full_backup->status == BACKUP_STATUS_MERGED)
@@ -253,11 +251,9 @@ do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool
 			}
 			if (!dest_backup)
 			{
-				char *tmp_backup_id = base36enc_dup(full_backup->start_time);
 				elog(WARNING, "Full backup %s has unfinished merge with missing backup %s",
-								tmp_backup_id,
+								base36enc(full_backup->start_time),
 								base36enc(full_backup->merge_dest_backup));
-				pg_free(tmp_backup_id);
 			}
 		}
 		else
@@ -344,10 +340,9 @@ do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool
 				full_backup->status == BACKUP_STATUS_MERGED) &&
 				dest_backup->start_time != full_backup->merge_dest_backup)
 			{
-				char *tmp_backup_id = base36enc_dup(full_backup->start_time);
 				elog(ERROR, "Full backup %s has unfinished merge with backup %s",
-					tmp_backup_id, base36enc(full_backup->merge_dest_backup));
-				pg_free(tmp_backup_id);
+					base36enc(full_backup->start_time),
+					base36enc(full_backup->merge_dest_backup));
 			}
 
 		}
@@ -441,7 +436,6 @@ merge_chain(InstanceState *instanceState,
 			bool no_validate, bool no_sync)
 {
 	int			i;
-	char 		*dest_backup_id;
 	char		full_external_prefix[MAXPGPATH];
 	char		full_database_dir[MAXPGPATH];
 	parray		*full_externals = NULL,
@@ -487,17 +481,11 @@ merge_chain(InstanceState *instanceState,
 	if (full_backup->merge_dest_backup != INVALID_BACKUP_ID &&
 		full_backup->merge_dest_backup != dest_backup->start_time)
 	{
-		char *merge_dest_backup_current = base36enc_dup(dest_backup->start_time);
-		char *merge_dest_backup = base36enc_dup(full_backup->merge_dest_backup);
-
 		elog(ERROR, "Cannot run merge for %s, because full backup %s has "
 					"unfinished merge with backup %s",
-			merge_dest_backup_current,
+			base36enc(dest_backup->start_time),
 			base36enc(full_backup->start_time),
-			merge_dest_backup);
-
-		pg_free(merge_dest_backup_current);
-		pg_free(merge_dest_backup);
+			base36enc(full_backup->merge_dest_backup));
 	}
 
 	/*
@@ -880,9 +868,9 @@ merge_chain(InstanceState *instanceState,
 	/*
 	 * Merging finished, now we can safely update ID of the FULL backup
 	 */
-	dest_backup_id = base36enc_dup(full_backup->merge_dest_backup);
 	elog(INFO, "Rename merged full backup %s to %s",
-				base36enc(full_backup->start_time), dest_backup_id);
+				base36enc(full_backup->start_time),
+				base36enc(full_backup->merge_dest_backup));
 
 	full_backup->status = BACKUP_STATUS_OK;
 	full_backup->start_time = full_backup->merge_dest_backup;
@@ -891,7 +879,6 @@ merge_chain(InstanceState *instanceState,
 	/* Critical section end */
 
 	/* Cleanup */
-	pg_free(dest_backup_id);
 	if (threads)
 	{
 		pfree(threads_args);

src/pg_probackup.h

@@ -1192,8 +1192,10 @@ extern void time2iso(char *buf, size_t len, time_t time, bool utc);
 extern const char *status2str(BackupStatus status);
 const char *status2str_color(BackupStatus status);
 extern BackupStatus str2status(const char *status);
-extern const char *base36enc(long unsigned int value);
-extern char *base36enc_dup(long unsigned int value);
+#define base36bufsize 14
+extern const char *base36enc_to(long unsigned int value, char buf[ARG_SIZE_HINT base36bufsize]);
+/* Abuse C99 Compound Literal's lifetime */
+#define base36enc(value) (base36enc_to((value), (char[base36bufsize]){0}))
 extern long unsigned int base36dec(const char *text);
 extern uint32 parse_server_version(const char *server_version_str);
 extern uint32 parse_program_version(const char *program_version);

src/restore.c

@@ -76,11 +76,11 @@ static void
 set_orphan_status(parray *backups, pgBackup *parent_backup)
 {
 	/* chain is intact, but at least one parent is invalid */
-	char	*parent_backup_id;
+	const char	*parent_backup_id;
 	int		j;
 
 	/* parent_backup_id is a human-readable backup ID  */
-	parent_backup_id = base36enc_dup(parent_backup->start_time);
+	parent_backup_id = base36enc(parent_backup->start_time);
 
 	for (j = 0; j < parray_num(backups); j++)
 	{
@@ -108,7 +108,6 @@ set_orphan_status(parray *backups, pgBackup *parent_backup)
 			}
 		}
 	}
-	pg_free(parent_backup_id);
 }
 
 /*
@@ -348,11 +347,11 @@ do_restore_or_validate(InstanceState *instanceState, time_t target_backup_id, pg
 			/* chain is broken, determine missing backup ID
 			 * and orphinize all his descendants
 			 */
-			char	   *missing_backup_id;
+			const char *missing_backup_id;
 			time_t		missing_backup_start_time;
 
 			missing_backup_start_time = tmp_backup->parent_backup;
-			missing_backup_id = base36enc_dup(tmp_backup->parent_backup);
+			missing_backup_id = base36enc(tmp_backup->parent_backup);
 
 			for (j = 0; j < parray_num(backups); j++)
 			{
@@ -363,22 +362,22 @@ do_restore_or_validate(InstanceState *instanceState, time_t target_backup_id, pg
 				 */
 				if (is_parent(missing_backup_start_time, backup, false))
 				{
+					const char	*backup_id = base36enc(backup->start_time);
 					if (backup->status == BACKUP_STATUS_OK ||
 						backup->status == BACKUP_STATUS_DONE)
 					{
 						write_backup_status(backup, BACKUP_STATUS_ORPHAN, true);
 
 						elog(WARNING, "Backup %s is orphaned because his parent %s is missing",
-								base36enc(backup->start_time), missing_backup_id);
+								backup_id, missing_backup_id);
 					}
 					else
 					{
 						elog(WARNING, "Backup %s has missing parent %s",
-								base36enc(backup->start_time), missing_backup_id);
+								backup_id, missing_backup_id);
 					}
 				}
 			}
-			pg_free(missing_backup_id);
 			/* No point in doing futher */
 			elog(ERROR, "%s of backup %s failed.", action, base36enc(dest_backup->start_time));
 		}

src/util.c

@@ -32,38 +32,23 @@ static const char *statusName[] =
 };
 
 const char *
-base36enc(long unsigned int value)
+base36enc_to(long unsigned int value, char buf[ARG_SIZE_HINT base36bufsize])
 {
 	const char	base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
 	/* log(2**64) / log(36) = 12.38 => max 13 char + '\0' */
-	static char	buffer[14];
-	unsigned int offset = sizeof(buffer);
+	char	buffer[14];
+	char   *p;
 
-	buffer[--offset] = '\0';
+	p = &buffer[sizeof(buffer)-1];
+	*p = '\0';
 	do {
-		buffer[--offset] = base36[value % 36];
+		*(--p) = base36[value % 36];
 	} while (value /= 36);
 
-	return &buffer[offset];
-}
-
-/*
- * Same as base36enc(), but the result must be released by the user.
- */
-char *
-base36enc_dup(long unsigned int value)
-{
-	const char	base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
-	/* log(2**64) / log(36) = 12.38 => max 13 char + '\0' */
-	char		buffer[14];
-	unsigned int offset = sizeof(buffer);
-
-	buffer[--offset] = '\0';
-	do {
-		buffer[--offset] = base36[value % 36];
-	} while (value /= 36);
+	/* I know, it doesn't look safe */
+	strncpy(buf, p, base36bufsize);
 
-	return strdup(&buffer[offset]);
+	return buf;
 }
 
 long unsigned int

src/utils/pgut.h

@@ -108,4 +108,10 @@ extern int sleep(unsigned int seconds);
 extern int usleep(unsigned int usec);
 #endif
 
+#ifdef _MSC_VER
+#define ARG_SIZE_HINT
+#else
+#define ARG_SIZE_HINT static
+#endif
+
 #endif   /* PGUT_H */