On Sun, Mar 27, 2022 at 1:47 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Coverity has a nitpick about this: > > /srv/coverity/git/pgsql-git/postgresql/src/common/backup_compression.c: 194 > in parse_bc_specification() > 193 /* Advance to next entry and loop around. */ > >>> CID 1503251: Null pointer dereferences (REVERSE_INULL) > >>> Null-checking "vend" suggests that it may be null, but it has already > >>> been dereferenced on all paths leading to the check. > 194 specification = vend == NULL ? kwend + 1 : vend + 1; > 195 } > 196 } > > Not sure if you should remove this null-check or add some other ones, > but I think you ought to do one or the other.
Yes, I think this is buggy. I think there's only a theoretical bug right now, because the only keyword we have is "level" and that requires a value. But if I add an example keyword that does not require an associated value (as demonstrated in the attached patch) and do something like pg_basebackup -cfast -D whatever --compress lz4:example, then the present code will dereference "vend" even though it's NULL, which is not good. The attached patch also shows how I think that should be fixed. As I hope is apparent, the first hunk of this patch is not for commit, and the second hunk is for commit. -- Robert Haas EDB: http://www.enterprisedb.com
coverity-backup-compression-fix.patch
Description: Binary data
