8000 base36enc: abuse C99 compound literals lifetime. · postgrespro/pg_probackup@22e6c40 · GitHub
[go: up one dir, main page]
Skip to content

Commit 22e6c40

Browse files
funny-falconfunny-falcon
committed
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.
1 parent eed2813 commit 22e6c40

File tree

9 files changed

+55
-90
lines changed

9 files changed

+55
-90
lines changed

‎src/backup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ do_backup(InstanceState *instanceState, pgSetBackupParams *set_backup_params,
735735
/* don't care about freeing base36enc_dup memory, we exit anyway */
736736
elog(ERROR, "Can't assign backup_id from requested start_time (%s), "
737737
"this time must be later that backup %s",
738-
base36enc_dup(start_time), base36enc_dup(latest_backup_id));
738+
base36enc(start_time), base36enc(latest_backup_id));
739739

740740
current.backup_id = start_time;
741741
pgBackupInitDir(&current, instanceState->instance_backup_subdir_path);

‎src/catalog.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,6 @@ catalog_get_last_data_backup(parray *backup_list, TimeLineID tli, time_t current
12191219
int i;
12201220
pgBackup *full_backup = NULL;
12211221
pgBackup *tmp_backup = NULL;
1222-
char *invalid_backup_id;
12231222

12241223
/* backup_list is sorted in order of descending ID */
12251224
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
12551254
{
12561255
/* broken chain */
12571256
case ChainIsBroken:
1258-
invalid_backup_id = base36enc_dup(tmp_backup->parent_backup);
1259-
12601257
elog(WARNING, "Backup %s has missing parent: %s. Cannot be a parent",
1261-
base36enc(backup->start_time), invalid_backup_id);
1262-
pg_free(invalid_backup_id);
1258+
base36enc(backup->start_time), base36enc(tmp_backup->parent_backup));
12631259
continue;
12641260

12651261
/* chain is intact, but at least one parent is invalid */
12661262
case ChainIsInvalid:
1267-
invalid_backup_id = base36enc_dup(tmp_backup->start_time);
1268-
12691263
elog(WARNING, "Backup %s has invalid parent: %s. Cannot be a parent",
1270-
base36enc(backup->start_time), invalid_backup_id);
1271-
pg_free(invalid_backup_id);
1264+
base36enc(backup->start_time), base36enc(tmp_backup->start_time));
12721265
continue;
12731266

12741267
/* chain is ok */

‎src/delete.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,6 @@ do_retention_merge(InstanceState *instanceState, parray *backup_list,
451451
/* Merging happens here */
452452
for (i = 0; i < parray_num(to_keep_list); i++)
453453
{
454-
char *keep_backup_id = NULL;
455454
pgBackup *full_backup = NULL;
456455
parray *merge_list = NULL;
457456

@@ -488,10 +487,9 @@ do_retention_merge(InstanceState *instanceState, parray *backup_list,
488487
* backups from purge_list.
489488
*/
490489

491-
keep_backup_id = base36enc_dup(keep_backup->start_time);
492490
elog(INFO, "Merge incremental chain between full backup %s and backup %s",
493-
base36enc(full_backup->start_time), keep_backup_id);
494-
pg_free(keep_backup_id);
491+
base36enc(full_backup->start_time),
492+
base36enc(keep_backup->start_time));
495493

496494
merge_list = parray_new();
497495

@@ -599,8 +597,6 @@ do_retention_purge(parray *to_keep_list, parray *to_purge_list)
599597
*/
600598
for (i = 0; i < parray_num(to_keep_list); i++)
601599
{
602-
char *keeped_backup_id;
603-
604600
pgBackup *keep_backup = (pgBackup *) parray_get(to_keep_list, i);
605601

606602
/* item could have been nullified in merge */
@@ -611,24 +607,22 @@ do_retention_purge(parray *to_keep_list, parray *to_purge_list)
611607
if (keep_backup->backup_mode == BACKUP_MODE_FULL)
612608
continue;
613609

614-
keeped_backup_id = base36enc_dup(keep_backup->start_time);
615-
616610
elog(LOG, "Check if backup %s is parent of backup %s",
617-
base36enc(delete_backup->start_time), keeped_backup_id);
611+
base36enc(delete_backup->start_time),
612+
base36enc(keep_backup->start_time));
618613

619614
if (is_parent(delete_backup->start_time, keep_backup, true))
620615
{
621616

622617
/* We must not delete this backup, evict it from purge list */
623618
elog(LOG, "Retain backup %s because his "
624619
"descendant %s is guarded by retention",
625-
base36enc(delete_backup->start_time), keeped_backup_id);
620+
base36enc(delete_backup->start_time),
621+
base36enc(keep_backup->start_time));
626622

627623
purge = false;
628-
pg_free(keeped_backup_id);
629624
break;
630625
}
631-
pg_free(keeped_backup_id);
632626
}
633627

634628
/* Retain backup */

‎src/merge.c

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,9 @@ do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool
223223
}
224224
if (!dest_backup)
225225
{
226-
char *tmp_backup_id = base36enc_dup(full_backup->start_time);
227226
elog(ERROR, "Full backup %s has unfinished merge with missing backup %s",
228-
tmp_backup_id,
227+
base36enc(full_backup->start_time),
229228
base36enc(full_backup->merge_dest_backup));
230-
pg_free(tmp_backup_id);
231229
}
232230
}
233231
else if (full_backup->status == BACKUP_STATUS_MERGED)
@@ -253,11 +251,9 @@ do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool
253251
}
254252
if (!dest_backup)
255253
{
256-
char *tmp_backup_id = base36enc_dup(full_backup->start_time);
257254
elog(WARNING, "Full backup %s has unfinished merge with missing backup %s",
258-
tmp_backup_id,
255+
base36enc(full_backup->start_time),
259256
base36enc(full_backup->merge_dest_backup));
260-
pg_free(tmp_backup_id);
261257
}
262258
}
263259
else
@@ -344,10 +340,9 @@ do_merge(InstanceState *instanceState, time_t backup_id, bool no_validate, bool
344340
full_backup->status == BACKUP_STATUS_MERGED) &&
345341
dest_backup->start_time != full_backup->merge_dest_backup)
346342
{
347-
char *tmp_backup_id = base36enc_dup(full_backup->start_time);
348343
elog(ERROR, "Full backup %s has unfinished merge with backup %s",
349-
tmp_backup_id, base36enc(full_backup->merge_dest_backup));
350-
pg_free(tmp_backup_id);
344+
base36enc(full_backup->start_time),
345+
base36enc(full_backup->merge_dest_backup));
351346
}
352347

353348
}
@@ -441,7 +436,6 @@ merge_chain(InstanceState *instanceState,
441436
bool no_validate, bool no_sync)
442437
{
443438
int i;
444-
char *dest_backup_id;
445439
char full_external_prefix[MAXPGPATH];
446440
char full_database_dir[MAXPGPATH];
447441
parray *full_externals = NULL,
@@ -487,17 +481,11 @@ merge_chain(InstanceState *instanceState,
487481
if (full_backup->merge_dest_backup != INVALID_BACKUP_ID &&
488482
full_backup->merge_dest_backup != dest_backup->start_time)
489483
{
490-
char *merge_dest_backup_current = base36enc_dup(dest_backup->start_time);
491-
char *merge_dest_backup = base36enc_dup(full_backup->merge_dest_backup);
492-
493484
elog(ERROR, "Cannot run merge for %s, because full backup %s has "
494485
"unfinished merge with backup %s",
495-
merge_dest_backup_current,
486+
base36enc(dest_backup->start_time),
496487
base36enc(full_backup->start_time),
497-
merge_dest_backup);
498-
499-
pg_free(merge_dest_backup_current);
500-
pg_free(merge_dest_backup);
488+
base36enc(full_backup->merge_dest_backup));
501489
}
502490

503491
/*
@@ -880,9 +868,9 @@ merge_chain(InstanceState *instanceState,
880868
/*
881869
* Merging finished, now we can safely update ID of the FULL backup
882870
*/
883-
dest_backup_id = base36enc_dup(full_backup->merge_dest_backup);
884871
elog(INFO, "Rename merged full backup %s to %s",
885-
base36enc(full_backup->start_time), dest_backup_id);
872+
base36enc(full_backup->start_time),
873+
base36enc(full_backup->merge_dest_backup));
886874

887875
full_backup->status = BACKUP_STATUS_OK;
888876
full_backup->start_time = full_backup->merge_dest_backup;
@@ -891,7 +879,6 @@ merge_chain(InstanceState *instanceState,
891879
/* Critical section end */
892880

893881
/* Cleanup */
894-
pg_free(dest_backup_id);
895882
if (threads)
896883
{
897884
pfree(threads_args);

‎src/pg_probackup.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,8 +1192,10 @@ extern void time2iso(char *buf, size_t len, time_t time, bool utc);
11921192
extern const char *status2str(BackupStatus status);
11931193
const char *status2str_color(BackupStatus status);
11941194
extern BackupStatus str2status(const char *status);
1195-
extern const char *base36enc(long unsigned int value);
1196-
extern char *base36enc_dup(long unsigned int value);
1195+
#define base36bufsize 14
1196+
extern const char *base36enc_to(long unsigned int value, char buf[ARG_SIZE_HINT base36bufsize]);
1197+
/* Abuse C99 Compound Literal's lifetime */
1198+
#define base36enc(value) (base36enc_to((value), (char[base36bufsize]){0}))
11971199
extern long unsigned int base36dec(const char *text);
11981200
extern uint32 parse_server_version(const char *server_version_str);
11991201
extern uint32 parse_program_version(const char *program_version);

‎src/restore.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,11 @@ static void
7676
set_orphan_status(parray *backups, pgBackup *parent_backup)
7777
{
7878
/* chain is intact, but at least one parent is invalid */
79-
char *parent_backup_id;
79+
const char *parent_backup_id;
8080
int j;
8181

8282
/* parent_backup_id is a human-readable backup ID */
83-
parent_backup_id = base36enc_dup(parent_backup->start_time);
83+
parent_backup_id = base36enc(parent_backup->start_time);
8484

8585
for (j = 0; j < parray_num(backups); j++)
8686
{
@@ -108,7 +108,6 @@ set_orphan_status(parray *backups, pgBackup *parent_backup)
108108
}
109109
}
110110
}
111-
pg_free(parent_backup_id);
112111
}
113112

114113
/*
@@ -348,11 +347,11 @@ do_restore_or_validate(InstanceState *instanceState, time_t target_backup_id, pg
348347
/* chain is broken, determine missing backup ID
349348
* and orphinize all his descendants
350349
*/
351-
char *missing_backup_id;
350+
const char *missing_backup_id;
352351
time_t missing_backup_start_time;
353352

354353
missing_backup_start_time = tmp_backup->parent_backup;
355-
missing_backup_id = base36enc_dup(tmp_backup->parent_backup);
354+
missing_backup_id = base36enc(tmp_backup->parent_backup);
356355

357356
for (j = 0; j < parray_num(backups); j++)
358357
{
@@ -363,22 +362,22 @@ do_restore_or_validate(InstanceState *instanceState, time_t target_backup_id, pg
363362
*/
364363
if (is_parent(missing_backup_start_time, backup, false))
365364
{
365+
const char *backup_id = base36enc(backup->start_time);
366366
if (backup->status == BACKUP_STATUS_OK ||
367367
backup->status == BACKUP_STATUS_DONE)
368368
{
369369
write_backup_status(backup, BACKUP_STATUS_ORPHAN, true);
370370

371371
elog(WARNING, "Backup %s is orphaned because his parent %s is missing",
372-
base36enc(backup->start_time), missing_backup_id);
372+
backup_id, missing_backup_id);
373373
}
374374
else
375375
{
376376
elog(WARNING, "Backup %s has missing parent %s",
377-
base36enc(backup->start_time), missing_backup_id);
377+
backup_id, missing_backup_id);
378378
}
379379
}
380380
}
381-
pg_free(missing_backup_id);
382381
/* No point in doing futher */
383382
elog(ERROR, "%s of backup %s failed.", action, base36enc(dest_backup->start_time));
384383
}

‎src/util.c

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,38 +32,23 @@ static const char *statusName[] =
3232
};
3333

3434
const char *
35-
base36enc(long unsigned int value)
35+
base36enc_to(long unsigned int value, char buf[ARG_SIZE_HINT base36bufsize])
3636
{
3737
const char base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
3838
/* log(2**64) / log(36) = 12.38 => max 13 char + '\0' */
39-
static char buffer[14];
40-
unsigned int offset = sizeof(buffer);
39+
char buffer[14];
40+
char *p;
4141

42-
buffer[--offset] = '\0';
42+
p = &buffer[sizeof(buffer)-1];
43+
*p = '\0';
4344
do {
44-
buffer[--offset] = base36[value % 36];
45+
*(--p) = base36[value % 36];
4546
} while (value /= 36);
4647

47-
return &buffer[offset];
48-
}
49-
50-
/*
51-
* Same as base36enc(), but the result must be released by the user.
52-
*/
53-
char *
54-
base36enc_dup(long unsigned int value)
55-
{
56-
const char base36[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
57-
/* log(2**64) / log(36) = 12.38 => max 13 char + '\0' */
58-
char buffer[14];
59-
unsigned int offset = sizeof(buffer);
60-
61-
buffer[--offset] = '\0';
62-
do {
63-
buffer[--offset] = base36[value % 36];
64-
} while (value /= 36);
48+
/* I know, it doesn't look safe */
49+
strncpy(buf, p, base36bufsize);
6550

66-
return strdup(&buffer[offset]);
51+
return buf;
6752
}
6853

6954
long unsigned int

‎src/utils/pgut.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,10 @@ extern int sleep(unsigned int seconds);
108108
extern int usleep(unsigned int usec);
109109
#endif
110110

111+
#ifdef _MSC_VER
112+
#define ARG_SIZE_HINT
113+
#else
114+
#define ARG_SIZE_HINT static
115+
#endif
116+
111117
#endif /* PGUT_H */

0 commit comments

Comments
 (0)
0