Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1w4UxA-002FpW-2l for pgsql-hackers@arkaria.postgresql.org; Mon, 23 Mar 2026 02:23:40 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1w4Ux9-00Exbk-0Y for pgsql-hackers@arkaria.postgresql.org; Mon, 23 Mar 2026 02:23:39 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1w4Ux8-00Exbc-2k for pgsql-hackers@lists.postgresql.org; Mon, 23 Mar 2026 02:23:39 +0000 Received: from mail-pf1-x42b.google.com ([2607:f8b0:4864:20::42b]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1w4Ux6-00000000aGq-2057 for pgsql-hackers@lists.postgresql.org; Mon, 23 Mar 2026 02:23:39 +0000 Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-82c4b5dfe6cso88925b3a.2 for ; Sun, 22 Mar 2026 19:23:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1774232613; x=1774837413; darn=lists.postgresql.org; h=message-id:in-reply-to:to:references:date:subject:mime-version:from :from:to:cc:subject:date:message-id:reply-to; bh=b4rzvaxI8fjkVP2FjUIfgCVwgCi+NXIC08busnykfeM=; b=RsZHHQhlBKn6i11+dEqFSOQCLAYuQEVGm/SFFEjSE/b0V2TymG+t1ynSNpnv2k7EAW Qd/iARi9+Itpgket7Q9jusZmF8NLi51GGNuQht9zPej5SAxDWTisdk7uV5KLMvxUw/7p Ilt6cz1f0FupETL+uOSVmVmmRMd92B1K67lZbag08c8wZ66jyN3WTzE/4j5c6Mxg2vl0 zwkwzvtSqjn2thwCLFkdTgftVKO2j9O5crwY9VsJVwWZKypcMiZlj7Q008KaonusnezL r5R79E+bfrmvtKNIYZIc69RRUomK4DFa45XaQF4s2oW9NPe1Xwg41vAjVurTKinFxcel CasA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774232613; x=1774837413; h=message-id:in-reply-to:to:references:date:subject:mime-version:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=b4rzvaxI8fjkVP2FjUIfgCVwgCi+NXIC08busnykfeM=; b=ZFF1ztPP9lnEYI2/bGhzd7ueksYP9KkwbJRJRYNWqYhKxLUF0C1wq21Us2BgUp26nX GiTnr77aPlWF0ihwfwrOytJ+2gh1QA3qm83QMgUQpSxrkQ3AswXVKgZTvIK6WLp9/10S oU2CvnCm8SKTAXHN44zQX7vDnzIPcZTIPOCN7K/rP3xuYg6SMm4LQYTx+V18glL6+oKv Gu2GK9d2DK0RFGcDR7zRNla07eXKLaES2Uz4kRYmI1ph5wlID2rXNzWeJAL/DszyMeeq TiFwF23HNGi+1O22Z3Avz6cOWKOlHrHvfujMl8CWa1rIF10A2qhphU8gn1XuRRi2NnXS 8bPQ== X-Gm-Message-State: AOJu0YxlNDN/n/C/9CHVRuzhbdYBKXRS+cC1JrFd+YvBXlnIrSCrK3Sf wtDfiEBIsc2DhsfiUuI0RwMBVrxqG90w4At0L4PkF96IUrRyugR2nDvtF0JGSsyG0fI= X-Gm-Gg: ATEYQzzsfTDMy0wEr1+8wrU5n+6L4hKwmC3QCTseaBPdZyb/cnxi+JETJdqawe/2zag uWsGuz3SkYkFmBESyOMRehTOsGmpBEfpFQYEbjHThrMNBbASfU5YN00Fitw66g2BQ5r71atyz5W a4RwVI0Xk/rnaVSSjhgxvWlQ/LWLN2FwNkQSDY12qf/SfbCFNs1otf46skJ6CqcKgCWV8TMurlq BYdgHkjB844kQjjqTZ8zOcyZBVL2KKj+aZDKi9nGZpfD0hR00pUJIlCorwvyPvwbeMh6QIcjQIo MqZMCvWDgkkuM92mHvv/G0/QFz2ngzfeFO+HjKDgk+Sb+4zwqwutrp9KhfpOhQSMjja7A2HBPMM 2N82xvmLT1/vYknrUoHyQ6BpEZmTxwIKnRwykkaXFIel+3PL00dSn+2/idfxt/HW/5vj8YNSEjY w6pYhhY5kyUrvnUhE8sV7qoY7aUpud+MVQxGLq8L/yLw== X-Received: by 2002:a05:6a00:9510:b0:82a:609f:d49c with SMTP id d2e1a72fcca58-82a8c35de95mr8086578b3a.47.1774232613244; Sun, 22 Mar 2026 19:23:33 -0700 (PDT) Received: from smtpclient.apple ([45.32.121.103]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-82b03aa7d8esm7685686b3a.4.2026.03.22.19.23.31 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 22 Mar 2026 19:23:32 -0700 (PDT) From: Chao Li Content-Type: multipart/mixed; boundary="Apple-Mail=_DA31DCF1-280B-4185-99DF-A0A1D136B2EC" Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\)) Subject: Re: Check some unchecked fclose() results Date: Mon, 23 Mar 2026 10:22:53 +0800 References: <191398CF-C65F-4751-B704-7C904BCBB80B@gmail.com> To: PostgreSQL Hackers In-Reply-To: <191398CF-C65F-4751-B704-7C904BCBB80B@gmail.com> Message-Id: X-Mailer: Apple Mail (2.3864.400.21) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --Apple-Mail=_DA31DCF1-280B-4185-99DF-A0A1D136B2EC Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Mar 23, 2026, at 10:04, Chao Li wrote: >=20 > Hi,=20 >=20 > This morning, while reading through recent commits, I noticed that = 69c57466a added a check for fclose(), with the explanation that =E2=80=9Cw= rite errors (like ENOSPC) may not be reported until close time.=E2=80=9D = More generally, an fclose() failure can also reflect an earlier buffered = write or flush failure that is only reported when the stream is closed. = So it seems worth checking these calls in other file-writing paths as = well. >=20 > So I did a quick search through the source tree, and I found several = other fclose() calls whose results are not checked. This patch fixes = some of them. >=20 > My criteria for including cases in this patch were basically: >=20 > * only output file descriptors > * code paths where the logic is relatively clear and easy to handle >=20 > If this patch gets processed, I would be happy to spend more time = handling the remaining cases. Or, if committers think all remaining = cases should be included in this patch, I would also be happy to expand = it. >=20 Forgot to attach the patch file. Here comes it. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ --Apple-Mail=_DA31DCF1-280B-4185-99DF-A0A1D136B2EC Content-Disposition: attachment; filename=v1-0001-Check-fclose-failures-in-more-places.patch Content-Type: application/octet-stream; x-unix-mode=0644; name="v1-0001-Check-fclose-failures-in-more-places.patch" Content-Transfer-Encoding: quoted-printable =46rom=209173313ee399247384abd9724c64fdf378dd49e5=20Mon=20Sep=2017=20= 00:00:00=202001=0AFrom:=20"Chao=20Li=20(Evan)"=20=0A= Date:=20Mon,=2023=20Mar=202026=2009:49:59=20+0800=0ASubject:=20[PATCH=20= v1]=20Check=20fclose()=20failures=20in=20more=20places=0A=0ACommit=20= 860359ea0=20fixed=20archive_waldump.c=20to=20check=20fclose(),=20noting=20= that=0Awrite=20errors=20such=20as=20ENOSPC=20may=20not=20be=20reported=20= until=20close=20time.=0A=0ADo=20the=20same=20in=20several=20other=20= file-writing=20paths=20that=20currently=20ignore=0Afclose()=20failures.=0A= =0AIn=20postmaster.c,=20if=20writing=20the=20external=20PID=20file=20= fails=20at=20fclose()=0Atime,=20report=20the=20error,=20avoid=20chmod()=20= on=20the=20incomplete=20file,=20and=0Aunlink=20it=20so=20that=20a=20= partial=20file=20is=20not=20left=20behind.=0A=0AAuthor:=20Chao=20Li=20= =0AReviewed-by:=0ADiscussion:=0A---=0A=20= src/backend/postmaster/postmaster.c=20=20=20=20=20=20=20=20=20|=2019=20= +++++++++++++------=0A=20src/bin/pg_basebackup/pg_createsubscriber.c=20|=20= =203=20++-=0A=20src/bin/pg_dump/pg_dumpall.c=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20=20|=2011=20++++++++---=0A=20= src/bin/pg_upgrade/pg_upgrade.c=20=20=20=20=20=20=20=20=20=20=20=20=20|=20= =203=20++-=0A=20src/fe_utils/astreamer_file.c=20=20=20=20=20=20=20=20=20=20= =20=20=20=20=20|=20=203=20++-=0A=20src/fe_utils/recovery_gen.c=20=20=20=20= =20=20=20=20=20=20=20=20=20=20=20=20=20|=20=206=20++++--=0A=206=20files=20= changed,=2031=20insertions(+),=2014=20deletions(-)=0A=0Adiff=20--git=20= a/src/backend/postmaster/postmaster.c=20= b/src/backend/postmaster/postmaster.c=0Aindex=203fac46c402b..95f20f35f31=20= 100644=0A---=20a/src/backend/postmaster/postmaster.c=0A+++=20= b/src/backend/postmaster/postmaster.c=0A@@=20-1300,12=20+1300,19=20@@=20= PostmasterMain(int=20argc,=20char=20*argv[])=0A=20=09=09if=20(fpidfile)=0A= =20=09=09{=0A=20=09=09=09fprintf(fpidfile,=20"%d\n",=20MyProcPid);=0A-=09= =09=09fclose(fpidfile);=0A-=0A-=09=09=09/*=20Make=20PID=20file=20world=20= readable=20*/=0A-=09=09=09if=20(chmod(external_pid_file,=20S_IRUSR=20|=20= S_IWUSR=20|=20S_IRGRP=20|=20S_IROTH)=20!=3D=200)=0A-=09=09=09=09= write_stderr("%s:=20could=20not=20change=20permissions=20of=20external=20= PID=20file=20\"%s\":=20%m\n",=0A+=09=09=09if=20(fclose(fpidfile)=20!=3D=20= 0)=0A+=09=09=09{=0A+=09=09=09=09write_stderr("%s:=20could=20not=20write=20= external=20PID=20file=20\"%s\":=20%m\n",=0A=20=09=09=09=09=09=09=09=20= progname,=20external_pid_file);=0A+=09=09=09=09= unlink(external_pid_file);=0A+=09=09=09}=0A+=09=09=09else=0A+=09=09=09{=0A= +=09=09=09=09/*=20Make=20PID=20file=20world=20readable=20*/=0A+=09=09=09=09= if=20(chmod(external_pid_file,=20S_IRUSR=20|=20S_IWUSR=20|=20S_IRGRP=20|=20= S_IROTH)=20!=3D=200)=0A+=09=09=09=09=09write_stderr("%s:=20could=20not=20= change=20permissions=20of=20external=20PID=20file=20\"%s\":=20%m\n",=0A+=09= =09=09=09=09=09=09=09=20progname,=20external_pid_file);=0A+=09=09=09}=0A=20= =09=09}=0A=20=09=09else=0A=20=09=09=09write_stderr("%s:=20could=20not=20= write=20external=20PID=20file=20\"%s\":=20%m\n",=0A@@=20-4117,7=20= +4124,7=20@@=20CreateOptsFile(int=20argc,=20char=20*argv[],=20char=20= *fullprogname)=0A=20=09=09fprintf(fp,=20"=20\"%s\"",=20argv[i]);=0A=20=09= fputs("\n",=20fp);=0A=20=0A-=09if=20(fclose(fp))=0A+=09if=20(fclose(fp)=20= !=3D=200)=0A=20=09{=0A=20=09=09ereport(LOG,=0A=20=09=09=09=09= (errcode_for_file_access(),=0Adiff=20--git=20= a/src/bin/pg_basebackup/pg_createsubscriber.c=20= b/src/bin/pg_basebackup/pg_createsubscriber.c=0Aindex=20= 2bc84505aab..27d2c830d4d=20100644=0A---=20= a/src/bin/pg_basebackup/pg_createsubscriber.c=0A+++=20= b/src/bin/pg_basebackup/pg_createsubscriber.c=0A@@=20-1390,7=20+1390,8=20= @@=20setup_recovery(const=20struct=20LogicalRepInfo=20*dbinfo,=20const=20= char=20*datadir,=20const=20c=0A=20=09=09if=20= (fwrite(recoveryconfcontents->data,=20recoveryconfcontents->len,=201,=20= fd)=20!=3D=201)=0A=20=09=09=09pg_fatal("could=20not=20write=20to=20file=20= \"%s\":=20%m",=20conf_filename);=0A=20=0A-=09=09fclose(fd);=0A+=09=09if=20= (fclose(fd)=20!=3D=200)=0A+=09=09=09pg_fatal("could=20not=20close=20file=20= \"%s\":=20%m",=20conf_filename);=0A=20=09=09recovery_params_set=20=3D=20= true;=0A=20=0A=20=09=09/*=20Include=20conditionally=20the=20recovery=20= parameters.=20*/=0Adiff=20--git=20a/src/bin/pg_dump/pg_dumpall.c=20= b/src/bin/pg_dump/pg_dumpall.c=0Aindex=2020cdd2d92f0..ba4831181c5=20= 100644=0A---=20a/src/bin/pg_dump/pg_dumpall.c=0A+++=20= b/src/bin/pg_dump/pg_dumpall.c=0A@@=20-842,7=20+842,8=20@@=20main(int=20= argc,=20char=20*argv[])=0A=20=0A=20=09=09if=20(filename)=0A=20=09=09{=0A= -=09=09=09fclose(OPF);=0A+=09=09=09if=20(fclose(OPF)=20!=3D=200)=0A+=09=09= =09=09pg_fatal("could=20not=20close=20output=20file=20\"%s\":=20%m",=20= filename);=0A=20=0A=20=09=09=09/*=20sync=20the=20resulting=20file,=20= errors=20are=20not=20fatal=20*/=0A=20=09=09=09if=20(dosync)=0A@@=20= -2099,7=20+2100,8=20@@=20dumpDatabases(PGconn=20*conn)=0A=20=09=09=09= create_opts=20=3D=20"--create";=0A=20=0A=20=09=09if=20(filename=20&&=20= archDumpFormat=20=3D=3D=20archNull)=0A-=09=09=09fclose(OPF);=0A+=09=09=09= if=20(fclose(OPF)=20!=3D=200)=0A+=09=09=09=09pg_fatal("could=20not=20= close=20output=20file=20\"%s\":=20%m",=20filename);=0A=20=0A=20=09=09/*=0A= =20=09=09=20*=20If=20this=20is=20not=20a=20plain=20format=20dump,=20then=20= append=20dboid=20and=20dbname=20to=0A@@=20-2133,7=20+2135,10=20@@=20= dumpDatabases(PGconn=20*conn)=0A=20=0A=20=09/*=20Close=20map=20file=20*/=0A= =20=09if=20(archDumpFormat=20!=3D=20archNull)=0A-=09=09fclose(map_file);=0A= +=09{=0A+=09=09if=20(fclose(map_file)=20!=3D=200)=0A+=09=09=09= pg_fatal("could=20not=20close=20map=20file:=20%m");=0A+=09}=0A=20=0A=20=09= PQclear(res);=0A=20}=0Adiff=20--git=20a/src/bin/pg_upgrade/pg_upgrade.c=20= b/src/bin/pg_upgrade/pg_upgrade.c=0Aindex=202127d297bfe..ee5dd654e25=20= 100644=0A---=20a/src/bin/pg_upgrade/pg_upgrade.c=0A+++=20= b/src/bin/pg_upgrade/pg_upgrade.c=0A@@=20-353,7=20+353,8=20@@=20= make_outputdirs(char=20*pgdata)=0A=20=09=09=09=09"=20=20pg_upgrade=20run=20= on=20%s"=0A=20=09=09=09=09= "-----------------------------------------------------------------\n\n",=0A= =20=09=09=09=09ctime(&run_time));=0A-=09=09fclose(fp);=0A+=09=09if=20= (fclose(fp)=20!=3D=200)=0A+=09=09=09pg_fatal("could=20not=20close=20log=20= file=20\"%s\":=20%m",=20filename_path);=0A=20=09}=0A=20}=0A=20=0Adiff=20= --git=20a/src/fe_utils/astreamer_file.c=20= b/src/fe_utils/astreamer_file.c=0Aindex=206e63a41af0d..33da0bc980b=20= 100644=0A---=20a/src/fe_utils/astreamer_file.c=0A+++=20= b/src/fe_utils/astreamer_file.c=0A@@=20-266,7=20+266,8=20@@=20= astreamer_extractor_content(astreamer=20*streamer,=20astreamer_member=20= *member,=0A=20=09=09case=20ASTREAMER_MEMBER_TRAILER:=0A=20=09=09=09if=20= (mystreamer->file=20=3D=3D=20NULL)=0A=20=09=09=09=09break;=0A-=09=09=09= fclose(mystreamer->file);=0A+=09=09=09if=20(fclose(mystreamer->file)=20= !=3D=200)=0A+=09=09=09=09pg_fatal("could=20not=20close=20file=20\"%s\":=20= %m",=20mystreamer->filename);=0A=20=09=09=09mystreamer->file=20=3D=20= NULL;=0A=20=09=09=09break;=0A=20=0Adiff=20--git=20= a/src/fe_utils/recovery_gen.c=20b/src/fe_utils/recovery_gen.c=0Aindex=20= f352652c785..f6da9e0d152=20100644=0A---=20a/src/fe_utils/recovery_gen.c=0A= +++=20b/src/fe_utils/recovery_gen.c=0A@@=20-143,7=20+143,8=20@@=20= WriteRecoveryConfig(PGconn=20*pgconn,=20const=20char=20*target_dir,=20= PQExpBuffer=20contents=0A=20=09if=20(fwrite(contents->data,=20= contents->len,=201,=20cf)=20!=3D=201)=0A=20=09=09pg_fatal("could=20not=20= write=20to=20file=20\"%s\":=20%m",=20filename);=0A=20=0A-=09fclose(cf);=0A= +=09if=20(fclose(cf)=20!=3D=200)=0A+=09=09pg_fatal("could=20not=20close=20= file=20\"%s\":=20%m",=20filename);=0A=20=0A=20=09if=20= (!use_recovery_conf)=0A=20=09{=0A@@=20-152,7=20+153,8=20@@=20= WriteRecoveryConfig(PGconn=20*pgconn,=20const=20char=20*target_dir,=20= PQExpBuffer=20contents=0A=20=09=09if=20(cf=20=3D=3D=20NULL)=0A=20=09=09=09= pg_fatal("could=20not=20create=20file=20\"%s\":=20%m",=20filename);=0A=20= =0A-=09=09fclose(cf);=0A+=09=09if=20(fclose(cf)=20!=3D=200)=0A+=09=09=09= pg_fatal("could=20not=20close=20file=20\"%s\":=20%m",=20filename);=0A=20=09= }=0A=20}=0A=20=0A--=20=0A2.50.1=20(Apple=20Git-155)=0A=0A= --Apple-Mail=_DA31DCF1-280B-4185-99DF-A0A1D136B2EC--