Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4fe2aa7

Browse files
committed
Reduce memory leakage in initdb.
While testing commit3e51b27, I noted that initdb leaks about amegabyte worth of data due to the sloppy bookkeeping in itsstring-manipulating code. That's not a huge amount on modern machines,but it's still kind of annoying, and it's easy to fix by recognizingthat we might as well treat these arrays of strings asmodifiable-in-place. There's no caller that cares about preservingthe old state of the array after replace_token or replace_guc_value.With this fix, valgrind sees only a few hundred bytes leaked duringan initdb run.Discussion:https://postgr.es/m/2844176.1674681919@sss.pgh.pa.us
1 parent3e51b27 commit4fe2aa7

File tree

1 file changed

+34
-53
lines changed

1 file changed

+34
-53
lines changed

‎src/bin/initdb/initdb.c

Lines changed: 34 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -402,47 +402,35 @@ add_stringlist_item(_stringlist **listhead, const char *str)
402402
}
403403

404404
/*
405-
*Make a copy ofthe array of lines,withtoken replacedby replacement
405+
*Modifythe array of lines,replacing "token"by"replacement"
406406
* the first time it occurs on each line.
407407
*
408-
* The original data structure is not changed, but we share any unchanged
409-
* strings with it. (This definition lends itself to memory leaks, but
410-
* we don't care too much about leaks in this program.)
408+
* The array must be a malloc'd array of individually malloc'd strings.
409+
* We free any discarded strings.
411410
*
412411
* This does most of what sed was used for in the shell script, but
413412
* doesn't need any regexp stuff.
414413
*/
415414
staticchar**
416415
replace_token(char**lines,constchar*token,constchar*replacement)
417416
{
418-
intnumlines=1;
419-
inti;
420-
char**result;
421417
inttoklen,
422418
replen,
423419
diff;
424420

425-
for (i=0;lines[i];i++)
426-
numlines++;
427-
428-
result= (char**)pg_malloc(numlines*sizeof(char*));
429-
430421
toklen=strlen(token);
431422
replen=strlen(replacement);
432423
diff=replen-toklen;
433424

434-
for (i=0;i<numlines;i++)
425+
for (inti=0;lines[i];i++)
435426
{
436427
char*where;
437428
char*newline;
438429
intpre;
439430

440-
/* just copy pointer if NULL or no change needed */
441-
if (lines[i]==NULL|| (where=strstr(lines[i],token))==NULL)
442-
{
443-
result[i]=lines[i];
431+
/* nothing to do if no change needed */
432+
if ((where=strstr(lines[i],token))==NULL)
444433
continue;
445-
}
446434

447435
/* if we get here a change is needed - set up new line */
448436

@@ -456,14 +444,15 @@ replace_token(char **lines, const char *token, const char *replacement)
456444

457445
strcpy(newline+pre+replen,lines[i]+pre+toklen);
458446

459-
result[i]=newline;
447+
free(lines[i]);
448+
lines[i]=newline;
460449
}
461450

462-
returnresult;
451+
returnlines;
463452
}
464453

465454
/*
466-
*Make a copy of the array of lines, replacing the possibly-commented-out
455+
*Modify the array of lines, replacing the possibly-commented-out
467456
* assignment of parameter guc_name with a live assignment of guc_value.
468457
* The value will be suitably quoted.
469458
*
@@ -474,18 +463,15 @@ replace_token(char **lines, const char *token, const char *replacement)
474463
* We assume there's at most one matching assignment. If we find no match,
475464
* append a new line with the desired assignment.
476465
*
477-
* The original data structure is not changed, but we share any unchanged
478-
* strings with it. (This definition lends itself to memory leaks, but
479-
* we don't care too much about leaks in this program.)
466+
* The array must be a malloc'd array of individually malloc'd strings.
467+
* We free any discarded strings.
480468
*/
481469
staticchar**
482470
replace_guc_value(char**lines,constchar*guc_name,constchar*guc_value,
483471
boolmark_as_comment)
484472
{
485-
char**result;
486473
intnamelen=strlen(guc_name);
487474
PQExpBuffernewline=createPQExpBuffer();
488-
intnumlines=0;
489475
inti;
490476

491477
/* prepare the replacement line, except for possible comment and newline */
@@ -497,17 +483,7 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
497483
else
498484
appendPQExpBufferStr(newline,guc_value);
499485

500-
/* create the new pointer array */
501486
for (i=0;lines[i];i++)
502-
numlines++;
503-
504-
/* leave room for one extra string in case we need to append */
505-
result= (char**)pg_malloc((numlines+2)*sizeof(char*));
506-
507-
/* initialize result with all the same strings */
508-
memcpy(result,lines, (numlines+1)*sizeof(char*));
509-
510-
for (i=0;i<numlines;i++)
511487
{
512488
constchar*where;
513489

@@ -517,7 +493,7 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
517493
* overrides a previous assignment. We allow leading whitespace too,
518494
* although normally there wouldn't be any.
519495
*/
520-
where=result[i];
496+
where=lines[i];
521497
while (*where=='#'||isspace((unsignedchar)*where))
522498
where++;
523499
if (strncmp(where,guc_name,namelen)!=0)
@@ -540,7 +516,7 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
540516
intoldindent=0;
541517
intnewindent;
542518

543-
for (ptr=result[i];ptr<where;ptr++)
519+
for (ptr=lines[i];ptr<where;ptr++)
544520
{
545521
if (*ptr=='\t')
546522
oldindent+=8- (oldindent %8);
@@ -573,23 +549,27 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
573549
else
574550
appendPQExpBufferChar(newline,'\n');
575551

576-
result[i]=newline->data;
552+
free(lines[i]);
553+
lines[i]=newline->data;
577554

578555
break;/* assume there's only one match */
579556
}
580557

581-
if (i >=numlines)
558+
if (lines[i]==NULL)
582559
{
583560
/*
584561
* No match, so append a new entry. (We rely on the bootstrap server
585562
* to complain if it's not a valid GUC name.)
586563
*/
587564
appendPQExpBufferChar(newline,'\n');
588-
result[numlines++]=newline->data;
589-
result[numlines]=NULL;/* keep the array null-terminated */
565+
lines=pg_realloc_array(lines,char*,i+2);
566+
lines[i++]=newline->data;
567+
lines[i]=NULL;/* keep the array null-terminated */
590568
}
591569

592-
returnresult;
570+
free(newline);/* but don't free newline->data */
571+
572+
returnlines;
593573
}
594574

595575
/*
@@ -626,6 +606,8 @@ guc_value_requires_quotes(const char *guc_value)
626606

627607
/*
628608
* get the lines from a text file
609+
*
610+
* The result is a malloc'd array of individually malloc'd strings.
629611
*/
630612
staticchar**
631613
readfile(constchar*path)
@@ -668,6 +650,9 @@ readfile(const char *path)
668650
/*
669651
* write an array of lines to a file
670652
*
653+
* "lines" must be a malloc'd array of individually malloc'd strings.
654+
* All that data is freed here.
655+
*
671656
* This is only used to write text files. Use fopen "w" not PG_BINARY_W
672657
* so that the resulting configuration files are nicely editable on Windows.
673658
*/
@@ -687,6 +672,7 @@ writefile(char *path, char **lines)
687672
}
688673
if (fclose(out_file))
689674
pg_fatal("could not close file \"%s\": %m",path);
675+
free(lines);
690676
}
691677

692678
/*
@@ -1218,7 +1204,6 @@ setup_config(void)
12181204
char**conflines;
12191205
charrepltok[MAXPGPATH];
12201206
charpath[MAXPGPATH];
1221-
char*autoconflines[3];
12221207
_stringlist*gnames,
12231208
*gvalues;
12241209

@@ -1384,18 +1369,17 @@ setup_config(void)
13841369
if (chmod(path,pg_file_create_mode)!=0)
13851370
pg_fatal("could not change permissions of \"%s\": %m",path);
13861371

1387-
free(conflines);
1388-
13891372

13901373
/* postgresql.auto.conf */
13911374

1392-
autoconflines[0]=pg_strdup("# Do not edit this file manually!\n");
1393-
autoconflines[1]=pg_strdup("# It will be overwritten by the ALTER SYSTEM command.\n");
1394-
autoconflines[2]=NULL;
1375+
conflines=pg_malloc_array(char*,3);
1376+
conflines[0]=pg_strdup("# Do not edit this file manually!\n");
1377+
conflines[1]=pg_strdup("# It will be overwritten by the ALTER SYSTEM command.\n");
1378+
conflines[2]=NULL;
13951379

13961380
sprintf(path,"%s/postgresql.auto.conf",pg_data);
13971381

1398-
writefile(path,autoconflines);
1382+
writefile(path,conflines);
13991383
if (chmod(path,pg_file_create_mode)!=0)
14001384
pg_fatal("could not change permissions of \"%s\": %m",path);
14011385

@@ -1466,7 +1450,6 @@ setup_config(void)
14661450
if (chmod(path,pg_file_create_mode)!=0)
14671451
pg_fatal("could not change permissions of \"%s\": %m",path);
14681452

1469-
free(conflines);
14701453

14711454
/* pg_ident.conf */
14721455

@@ -1478,8 +1461,6 @@ setup_config(void)
14781461
if (chmod(path,pg_file_create_mode)!=0)
14791462
pg_fatal("could not change permissions of \"%s\": %m",path);
14801463

1481-
free(conflines);
1482-
14831464
check_ok();
14841465
}
14851466

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp