Skip to content

Commit 80ae841

Browse files
alvherrealvherre
committed
Measure string lengths only once
Bernd Helmle complained that CreateReplicationSlot() was assigning the same value to the same variable twice, so we could remove one of them. Code inspection reveals that we can actually remove both assignments: according to the author the assignment was there for beauty of the strlen line only, and another possible fix to that is to put the strlen in its own line, so do that. To be consistent within the file, refactor all duplicated strlen() calls, which is what we do elsewhere in the backend anyway. In basebackup.c, snprintf already returns the right length; no need for strlen afterwards. Backpatch to 9.4, where replication slots were introduced, to keep code identical. Some of this is older, but the patch doesn't apply cleanly and it's only of cosmetic value anyway. Discussion: http://www.postgresql.org/message-id/BE2FD71DEA35A2287EA5F018@eje.credativ.lan
1 parent 44390e3 commit 80ae841

File tree

2 files changed

+70
-45
lines changed

2 files changed

+70
-45
lines changed

‎src/backend/replication/basebackup.c

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -698,10 +698,15 @@ SendBackupHeader(List *tablespaces)
698698
}
699699
else
700700
{
701-
pq_sendint(&buf, strlen(ti->oid), 4); /* length */
702-
pq_sendbytes(&buf, ti->oid, strlen(ti->oid));
703-
pq_sendint(&buf, strlen(ti->path), 4); /* length */
704-
pq_sendbytes(&buf, ti->path, strlen(ti->path));
701+
Size len;
702+
703+
len = strlen(ti->oid);
704+
pq_sendint(&buf, len, 4);
705+
pq_sendbytes(&buf, ti->oid, len);
706+
707+
len = strlen(ti->path);
708+
pq_sendint(&buf, len, 4);
709+
pq_sendbytes(&buf, ti->path, len);
705710
}
706711
if (ti->size >= 0)
707712
send_int8_string(&buf, ti->size / 1024);
@@ -724,6 +729,7 @@ SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli)
724729
{
725730
StringInfoData buf;
726731
char str[MAXFNAMELEN];
732+
Size len;
727733

728734
pq_beginmessage(&buf, 'T'); /* RowDescription */
729735
pq_sendint(&buf, 2, 2); /* 2 fields */
@@ -742,7 +748,7 @@ SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli)
742748
pq_sendint(&buf, 0, 2); /* attnum */
743749

744750
/*
745-
* int8 may seem like a surprising data type for this, but in thory int4
751+
* int8 may seem like a surprising data type for this, but in theory int4
746752
* would not be wide enough for this, as TimeLineID is unsigned.
747753
*/
748754
pq_sendint(&buf, INT8OID, 4); /* type oid */
@@ -755,13 +761,15 @@ SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli)
755761
pq_beginmessage(&buf, 'D');
756762
pq_sendint(&buf, 2, 2); /* number of columns */
757763

758-
snprintf(str, sizeof(str), "%X/%X", (uint32) (ptr >> 32), (uint32) ptr);
759-
pq_sendint(&buf, strlen(str), 4); /* length */
760-
pq_sendbytes(&buf, str, strlen(str));
764+
len = snprintf(str, sizeof(str),
765+
"%X/%X", (uint32) (ptr >> 32), (uint32) ptr);
766+
pq_sendint(&buf, len, 4);
767+
pq_sendbytes(&buf, str, len);
768+
769+
len = snprintf(str, sizeof(str), "%u", tli);
770+
pq_sendint(&buf, len, 4);
771+
pq_sendbytes(&buf, str, len);
761772

762-
snprintf(str, sizeof(str), "%u", tli);
763-
pq_sendint(&buf, strlen(str), 4); /* length */
764-
pq_sendbytes(&buf, str, strlen(str));
765773
pq_endmessage(&buf);
766774

767775
/* Send a CommandComplete message */

‎src/backend/replication/walsender.c

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ IdentifySystem(void)
299299
char xpos[MAXFNAMELEN];
300300
XLogRecPtr logptr;
301301
char *dbname = NULL;
302+
Size len;
302303

303304
/*
304305
* Reply with a result set with one row, four columns. First col is system
@@ -380,21 +381,32 @@ IdentifySystem(void)
380381
/* Send a DataRow message */
381382
pq_beginmessage(&buf, 'D');
382383
pq_sendint(&buf, 4, 2); /* # of columns */
383-
pq_sendint(&buf, strlen(sysid), 4); /* col1 len */
384-
pq_sendbytes(&buf, (char *) &sysid, strlen(sysid));
385-
pq_sendint(&buf, strlen(tli), 4); /* col2 len */
386-
pq_sendbytes(&buf, (char *) tli, strlen(tli));
387-
pq_sendint(&buf, strlen(xpos), 4); /* col3 len */
388-
pq_sendbytes(&buf, (char *) xpos, strlen(xpos));
389-
/* send NULL if not connected to a database */
384+
385+
/* column 1: system identifier */
386+
len = strlen(sysid);
387+
pq_sendint(&buf, len, 4);
388+
pq_sendbytes(&buf, (char *) &sysid, len);
389+
390+
/* column 2: timeline */
391+
len = strlen(tli);
392+
pq_sendint(&buf, len, 4);
393+
pq_sendbytes(&buf, (char *) tli, len);
394+
395+
/* column 3: xlog position */
396+
len = strlen(xpos);
397+
pq_sendint(&buf, len, 4);
398+
pq_sendbytes(&buf, (char *) xpos, len);
399+
400+
/* column 4: database name, or NULL if none */
390401
if (dbname)
391402
{
392-
pq_sendint(&buf, strlen(dbname), 4); /* col4 len */
393-
pq_sendbytes(&buf, (char *) dbname, strlen(dbname));
403+
len = strlen(dbname);
404+
pq_sendint(&buf, len, 4);
405+
pq_sendbytes(&buf, (char *) dbname, len);
394406
}
395407
else
396408
{
397-
pq_sendint(&buf, -1, 4); /* col4 len, NULL */
409+
pq_sendint(&buf, -1, 4);
398410
}
399411

400412
pq_endmessage(&buf);
@@ -413,6 +425,7 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
413425
int fd;
414426
off_t histfilelen;
415427
off_t bytesleft;
428+
Size len;
416429

417430
/*
418431
* Reply with a result set with one row, and two columns. The first col is
@@ -448,8 +461,9 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd)
448461
/* Send a DataRow message */
449462
pq_beginmessage(&buf, 'D');
450463
pq_sendint(&buf, 2, 2); /* # of columns */
451-
pq_sendint(&buf, strlen(histfname), 4); /* col1 len */
452-
pq_sendbytes(&buf, histfname, strlen(histfname));
464+
len = strlen(histfname);
465+
pq_sendint(&buf, len, 4); /* col1 len */
466+
pq_sendbytes(&buf, histfname, len);
453467

454468
fd = OpenTransientFile(path, O_RDONLY | PG_BINARY, 0666);
455469
if (fd < 0)
@@ -675,6 +689,7 @@ StartReplication(StartReplicationCmd *cmd)
675689
{
676690
char tli_str[11];
677691
char startpos_str[8 + 1 + 8 + 1];
692+
Size len;
678693

679694
snprintf(tli_str, sizeof(tli_str), "%u", sendTimeLineNextTLI);
680695
snprintf(startpos_str, sizeof(startpos_str), "%X/%X",
@@ -711,11 +726,13 @@ StartReplication(StartReplicationCmd *cmd)
711726
pq_beginmessage(&buf, 'D');
712727
pq_sendint(&buf, 2, 2); /* number of columns */
713728

714-
pq_sendint(&buf, strlen(tli_str), 4); /* length */
715-
pq_sendbytes(&buf, tli_str, strlen(tli_str));
729+
len = strlen(tli_str);
730+
pq_sendint(&buf, len, 4); /* length */
731+
pq_sendbytes(&buf, tli_str, len);
716732

717-
pq_sendint(&buf, strlen(startpos_str), 4); /* length */
718-
pq_sendbytes(&buf, startpos_str, strlen(startpos_str));
733+
len = strlen(startpos_str);
734+
pq_sendint(&buf, len, 4); /* length */
735+
pq_sendbytes(&buf, startpos_str, len);
719736

720737
pq_endmessage(&buf);
721738
}
@@ -763,10 +780,10 @@ logical_read_xlog_page(XLogReaderState *state, XLogRecPtr targetPagePtr, int req
763780
static void
764781
CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
765782
{
766-
const char *slot_name;
767783
const char *snapshot_name = NULL;
768784
char xpos[MAXFNAMELEN];
769785
StringInfoData buf;
786+
Size len;
770787

771788
Assert(!MyReplicationSlot);
772789

@@ -792,14 +809,11 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
792809

793810
initStringInfo(&output_message);
794811

795-
slot_name = NameStr(MyReplicationSlot->data.name);
796-
797812
if (cmd->kind == REPLICATION_KIND_LOGICAL)
798813
{
799814
LogicalDecodingContext *ctx;
800815

801-
ctx = CreateInitDecodingContext(
802-
cmd->plugin, NIL,
816+
ctx = CreateInitDecodingContext(cmd->plugin, NIL,
803817
logical_read_xlog_page,
804818
WalSndPrepareWrite, WalSndWriteData);
805819

@@ -827,7 +841,6 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
827841
ReplicationSlotPersist();
828842
}
829843

830-
slot_name = NameStr(MyReplicationSlot->data.name);
831844
snprintf(xpos, sizeof(xpos), "%X/%X",
832845
(uint32) (MyReplicationSlot->data.confirmed_flush >> 32),
833846
(uint32) MyReplicationSlot->data.confirmed_flush);
@@ -878,30 +891,34 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
878891
pq_sendint(&buf, 4, 2); /* # of columns */
879892

880893
/* slot_name */
881-
pq_sendint(&buf, strlen(slot_name), 4); /* col1 len */
882-
pq_sendbytes(&buf, slot_name, strlen(slot_name));
894+
len = strlen(NameStr(MyReplicationSlot->data.name));
895+
pq_sendint(&buf, len, 4); /* col1 len */
896+
pq_sendbytes(&buf, NameStr(MyReplicationSlot->data.name), len);
883897

884898
/* consistent wal location */
885-
pq_sendint(&buf, strlen(xpos), 4); /* col2 len */
886-
pq_sendbytes(&buf, xpos, strlen(xpos));
899+
len = strlen(xpos);
900+
pq_sendint(&buf, len, 4);
901+
pq_sendbytes(&buf, xpos, len);
887902

888-
/* snapshot name */
903+
/* snapshot name, or NULL if none */
889904
if (snapshot_name != NULL)
890905
{
891-
pq_sendint(&buf, strlen(snapshot_name), 4); /* col3 len */
892-
pq_sendbytes(&buf, snapshot_name, strlen(snapshot_name));
906+
len = strlen(snapshot_name);
907+
pq_sendint(&buf, len, 4);
908+
pq_sendbytes(&buf, snapshot_name, len);
893909
}
894910
else
895-
pq_sendint(&buf, -1, 4); /* col3 len, NULL */
911+
pq_sendint(&buf, -1, 4);
896912

897-
/* plugin */
913+
/* plugin, or NULL if none */
898914
if (cmd->plugin != NULL)
899915
{
900-
pq_sendint(&buf, strlen(cmd->plugin), 4); /* col4 len */
901-
pq_sendbytes(&buf, cmd->plugin, strlen(cmd->plugin));
916+
len = strlen(cmd->plugin);
917+
pq_sendint(&buf, len, 4);
918+
pq_sendbytes(&buf, cmd->plugin, len);
902919
}
903920
else
904-
pq_sendint(&buf, -1, 4); /* col4 len, NULL */
921+
pq_sendint(&buf, -1, 4);
905922

906923
pq_endmessage(&buf);
907924

0 commit comments

Comments
 (0)