Fixing code that ignores failure of XLogRecGetBlockTag
Currently, XLogRecGetBlockTag has 41 callers, of which only four
bother to check the function's result. The remainder take it on
faith that they got valid data back, and many of them will
misbehave in seriously nasty ways if they didn't. (This point
was drawn to my attention by a Coverity complaint.)
I think we should make this a little less fragile. Since we
already have XLogRecGetBlockTagExtended, I propose that callers
that need to handle the case of no-such-block must use that,
while XLogRecGetBlockTag throws an error. The attached patch
fixes that up, and also cleans up some random inconsistency
about use of XLogRecHasBlockRef().
regards, tom lane
Attachments:
make-XLogRecGetBlockTag-throw-error.patchtext/x-diff; charset=us-ascii; name=make-XLogRecGetBlockTag-throw-error.patchDownload+36-23
On Mon, Apr 11, 2022 at 2:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Currently, XLogRecGetBlockTag has 41 callers, of which only four
bother to check the function's result. The remainder take it on
faith that they got valid data back, and many of them will
misbehave in seriously nasty ways if they didn't. (This point
was drawn to my attention by a Coverity complaint.)I think we should make this a little less fragile. Since we
already have XLogRecGetBlockTagExtended, I propose that callers
that need to handle the case of no-such-block must use that,
while XLogRecGetBlockTag throws an error. The attached patch
fixes that up, and also cleans up some random inconsistency
about use of XLogRecHasBlockRef().
Looks reasonable.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Apr 12, 2022 at 8:58 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 2:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Currently, XLogRecGetBlockTag has 41 callers, of which only four
bother to check the function's result. The remainder take it on
faith that they got valid data back, and many of them will
misbehave in seriously nasty ways if they didn't. (This point
was drawn to my attention by a Coverity complaint.)I think we should make this a little less fragile. Since we
already have XLogRecGetBlockTagExtended, I propose that callers
that need to handle the case of no-such-block must use that,
while XLogRecGetBlockTag throws an error. The attached patch
fixes that up, and also cleans up some random inconsistency
about use of XLogRecHasBlockRef().Looks reasonable.
+1
Thomas Munro <thomas.munro@gmail.com> writes:
On Tue, Apr 12, 2022 at 8:58 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Apr 11, 2022 at 2:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I think we should make this a little less fragile. Since we
already have XLogRecGetBlockTagExtended, I propose that callers
that need to handle the case of no-such-block must use that,
while XLogRecGetBlockTag throws an error. The attached patch
fixes that up, and also cleans up some random inconsistency
about use of XLogRecHasBlockRef().
Looks reasonable.
+1
Pushed, thanks for looking.
regards, tom lane