From b1c169caf0678a82cf26b5656e01399f6153456b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 19 Dec 2021 17:18:34 -0500 Subject: [PATCH] Remove some more dead code in pg_dump. Coverity complained that parts of dumpFunc() and buildACLCommands() were now unreachable, as indeed they are. Remove 'em. In passing, make dumpFunc's handling of protrftypes less gratuitously different from other fields. --- src/bin/pg_dump/dumputils.c | 72 ++++++++++--------------------------- src/bin/pg_dump/pg_dump.c | 72 ++++--------------------------------- 2 files changed, 25 insertions(+), 119 deletions(-) diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index b5900e4ed8..a2b9fe7e35 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -122,10 +122,6 @@ buildACLCommands(const char *name, const char *subname, const char *nspname, * strings are the work of aclitemout(), it should be OK in practice. * Besides, a false mismatch will just cause the output to be a little * more verbose than it really needed to be. - * - * (If we weren't given a base ACL, this stanza winds up with all the - * ACL's items in grantitems and nothing in revokeitems. It's not worth - * special-casing that.) */ grantitems = (char **) pg_malloc(naclitems * sizeof(char *)); for (i = 0; i < naclitems; i++) @@ -173,60 +169,30 @@ buildACLCommands(const char *name, const char *subname, const char *nspname, secondsql = createPQExpBuffer(); /* - * If we weren't given baseacls information, we just revoke everything and - * then grant what's listed in the ACL. This avoids having to embed - * detailed knowledge about what the defaults are/were, and it's not very - * expensive since servers lacking acldefault() are now rare. - * - * Otherwise, we need only revoke what's listed in revokeitems. + * Build REVOKE statements for ACLs listed in revokeitems[]. */ - if (baseacls == NULL || *baseacls == '\0') + for (i = 0; i < nrevokeitems; i++) { - /* We assume the old defaults only involved the owner and PUBLIC */ - appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix); - if (subname) - appendPQExpBuffer(firstsql, "(%s)", subname); - appendPQExpBuffer(firstsql, " ON %s ", type); - if (nspname && *nspname) - appendPQExpBuffer(firstsql, "%s.", fmtId(nspname)); - appendPQExpBuffer(firstsql, "%s FROM PUBLIC;\n", name); - if (owner) + if (!parseAclItem(revokeitems[i], + type, name, subname, remoteVersion, + grantee, grantor, privs, NULL)) { - appendPQExpBuffer(firstsql, "%sREVOKE ALL", prefix); - if (subname) - appendPQExpBuffer(firstsql, "(%s)", subname); - appendPQExpBuffer(firstsql, " ON %s ", type); + ok = false; + break; + } + + if (privs->len > 0) + { + appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ", + prefix, privs->data, type); if (nspname && *nspname) appendPQExpBuffer(firstsql, "%s.", fmtId(nspname)); - appendPQExpBuffer(firstsql, "%s FROM %s;\n", name, fmtId(owner)); - } - } - else - { - /* Scan individual REVOKE ACL items */ - for (i = 0; i < nrevokeitems; i++) - { - if (!parseAclItem(revokeitems[i], - type, name, subname, remoteVersion, - grantee, grantor, privs, NULL)) - { - ok = false; - break; - } - - if (privs->len > 0) - { - appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ", - prefix, privs->data, type); - if (nspname && *nspname) - appendPQExpBuffer(firstsql, "%s.", fmtId(nspname)); - appendPQExpBuffer(firstsql, "%s FROM ", name); - if (grantee->len == 0) - appendPQExpBufferStr(firstsql, "PUBLIC;\n"); - else - appendPQExpBuffer(firstsql, "%s;\n", - fmtId(grantee->data)); - } + appendPQExpBuffer(firstsql, "%s FROM ", name); + if (grantee->len == 0) + appendPQExpBufferStr(firstsql, "PUBLIC;\n"); + else + appendPQExpBuffer(firstsql, "%s;\n", + fmtId(grantee->data)); } } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 784771c2d7..ac291fbef2 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -11305,9 +11305,6 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) char *funcargs; char *funciargs; char *funcresult; - char *proallargtypes; - char *proargmodes; - char *proargnames; char *protrftypes; char *prokind; char *provolatile; @@ -11320,10 +11317,6 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) char *prosupport; char *proparallel; char *lanname; - int nallargs; - char **allargtypes = NULL; - char **argmodes = NULL; - char **argnames = NULL; char **configitems = NULL; int nconfigitems = 0; const char *keyword; @@ -11364,6 +11357,9 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) if (fout->remoteVersion >= 90500) appendPQExpBufferStr(query, "array_to_string(protrftypes, ' ') AS protrftypes,\n"); + else + appendPQExpBufferStr(query, + "NULL AS protrftypes,\n"); if (fout->remoteVersion >= 90600) appendPQExpBufferStr(query, @@ -11425,11 +11421,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) funcargs = PQgetvalue(res, 0, PQfnumber(res, "funcargs")); funciargs = PQgetvalue(res, 0, PQfnumber(res, "funciargs")); funcresult = PQgetvalue(res, 0, PQfnumber(res, "funcresult")); - proallargtypes = proargmodes = proargnames = NULL; - if (PQfnumber(res, "protrftypes") != -1) - protrftypes = PQgetvalue(res, 0, PQfnumber(res, "protrftypes")); - else - protrftypes = NULL; + protrftypes = PQgetvalue(res, 0, PQfnumber(res, "protrftypes")); prokind = PQgetvalue(res, 0, PQfnumber(res, "prokind")); provolatile = PQgetvalue(res, 0, PQfnumber(res, "provolatile")); proisstrict = PQgetvalue(res, 0, PQfnumber(res, "proisstrict")); @@ -11479,53 +11471,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) appendStringLiteralDQ(asPart, prosrc, NULL); } - nallargs = finfo->nargs; /* unless we learn different from allargs */ - - if (proallargtypes && *proallargtypes) - { - int nitems = 0; - - if (!parsePGArray(proallargtypes, &allargtypes, &nitems) || - nitems < finfo->nargs) - { - pg_log_warning("could not parse %s array", "proallargtypes"); - if (allargtypes) - free(allargtypes); - allargtypes = NULL; - } - else - nallargs = nitems; - } - - if (proargmodes && *proargmodes) - { - int nitems = 0; - - if (!parsePGArray(proargmodes, &argmodes, &nitems) || - nitems != nallargs) - { - pg_log_warning("could not parse %s array", "proargmodes"); - if (argmodes) - free(argmodes); - argmodes = NULL; - } - } - - if (proargnames && *proargnames) - { - int nitems = 0; - - if (!parsePGArray(proargnames, &argnames, &nitems) || - nitems != nallargs) - { - pg_log_warning("could not parse %s array", "proargnames"); - if (argnames) - free(argnames); - argnames = NULL; - } - } - - if (proconfig && *proconfig) + if (*proconfig) { if (!parsePGArray(proconfig, &configitems, &nconfigitems)) fatal("could not parse %s array", "proconfig"); @@ -11571,7 +11517,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) appendPQExpBuffer(q, "\n LANGUAGE %s", fmtId(lanname)); - if (protrftypes != NULL && strcmp(protrftypes, "") != 0) + if (*protrftypes) { Oid *typeids = palloc(FUNC_MAX_ARGS * sizeof(Oid)); int i; @@ -11749,12 +11695,6 @@ dumpFunc(Archive *fout, const FuncInfo *finfo) free(funcfullsig); free(funcsig_tag); free(qual_funcsig); - if (allargtypes) - free(allargtypes); - if (argmodes) - free(argmodes); - if (argnames) - free(argnames); if (configitems) free(configitems); }