Daniel Gustafsson <daniel@yesql.se> writes:
> I spent a few more hours staring at this, and ran it through a number of CI and
> local builds, without anything showing up. Pushed to master with the first set
> of buildfarm animals showing green builds.
Coverity has a nit-pick about this:
/srv/coverity/git/pgsql-git/postgresql/src/interfaces/libpq/fe-auth-oauth.c: 784 in setup_token_request()
778 if (!request_copy)
779 {
780 libpq_append_conn_error(conn, "out of memory");
781 goto fail;
782 }
783
>>> CID 1643156: High impact quality (WRITE_CONST_FIELD)
>>> A write to an aggregate overwrites a const-qualified field within the aggregate.
784 memcpy(request_copy, &request, sizeof(request));
785
786 conn->async_auth = run_user_oauth_flow;
787 conn->cleanup_async_auth = cleanup_user_oauth_flow;
788 state->async_ctx = request_copy;
789 }
This is evidently because of the fields declared const:
/* Hook inputs (constant across all calls) */
const char *const openid_configuration; /* OIDC discovery URI */
const char *const scope; /* required scope(s), or NULL */
IMO, the set of cases where it's legitimate to mark individual struct
fields as const is negligibly small, and this doesn't seem to be one
of them. It's not obvious to me where/how PGoauthBearerRequest
structs are supposed to be constructed, but I find it hard to believe
that they will all spring full-grown from the forehead of Zeus.
Nonetheless, this declaration requires exactly that.
(I'm kind of surprised that we're not getting similar bleats from
any buildfarm animals, but so far I don't see any.)
BTW, as another nitpicky style matter: why do PGoauthBearerRequest
etc. spell their struct tag names differently from their typedef names
(that is, with/without an underscore)? That is not our project style
anywhere else, and I'm failing to detect a good reason to do it here.
regards, tom lane
