On Wed, Apr 7, 2021 at 1:24 PM Magnus Hagander <mag...@hagander.net> wrote:
>
> On Wed, Apr 7, 2021 at 1:01 PM Erik Rijkers <e...@xs4all.nl> wrote:
> >
> > Recently (last day or so), I get this warning from gcc 10.2:
> >
> > -----
> > hba.c:3160:18: warning: comparison of unsigned enum expression < 0 is
> > always false [-Wtautological-compare]
> > if (auth_method < 0 || USER_AUTH_LAST < auth_method)
> > ~~~~~~~~~~~ ^ ~
> > 1 warning generated.
> > -----
>
> This one is from 9afffcb833d3c5e59a328a2af674fac7e7334fc1 (adding
> Jacob and Michael to cc)
>
> And it makes sense to give warning on that. AuthMethod is an enum. It
> can by definition not have a value that's not in the enum. That check
> simply seems wrong/unnecessary.
>
> The only other use fo USER_AUTH_LAST is in fill_hba_line() which also
> gets the name of the auth. That one uses :
> StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
> "UserAuthName[] must match the UserAuth enum");
>
> Which seems like a more reasonable check.
>
> But that also highlights -- shouldn't that function then also be made
> to use hba_authname(), and the assert moved into the function? That
> seems like the cleanest way?
So to be clear, this is what I'm suggesting.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index dee056b0d6..27865b14a0 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -379,7 +379,7 @@ set_authn_id(Port *port, const char *id)
ereport(LOG,
errmsg("connection authenticated: identity=\"%s\" method=%s "
"(%s:%d)",
- port->authn_id, hba_authname(port), HbaFileName,
+ port->authn_id, hba_authname(port->hba->auth_method), HbaFileName,
port->hba->linenumber));
}
}
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index b720b03e9a..60767f2957 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2607,14 +2607,8 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
else
nulls[index++] = true;
- /*
- * Make sure UserAuthName[] tracks additions to the UserAuth enum
- */
- StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
- "UserAuthName[] must match the UserAuth enum");
-
/* auth_method */
- values[index++] = CStringGetTextDatum(UserAuthName[hba->auth_method]);
+ values[index++] = CStringGetTextDatum(hba_authname(hba->auth_method));
/* options */
options = gethba_options(hba);
@@ -3150,18 +3144,13 @@ hba_getauthmethod(hbaPort *port)
* should not be freed.
*/
const char *
-hba_authname(hbaPort *port)
+hba_authname(UserAuth auth_method)
{
- UserAuth auth_method;
-
- Assert(port->hba);
- auth_method = port->hba->auth_method;
-
- if (auth_method < 0 || USER_AUTH_LAST < auth_method)
- {
- /* Should never happen. */
- elog(FATAL, "port has out-of-bounds UserAuth: %d", auth_method);
- }
+ /*
+ * Make sure UserAuthName[] tracks additions to the UserAuth enum
+ */
+ StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
+ "UserAuthName[] must match the UserAuth enum");
return UserAuthName[auth_method];
}
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 63f2962139..8d9f3821b1 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -137,7 +137,7 @@ typedef struct Port hbaPort;
extern bool load_hba(void);
extern bool load_ident(void);
-extern const char *hba_authname(hbaPort *port);
+extern const char *hba_authname(UserAuth auth_method);
extern void hba_getauthmethod(hbaPort *port);
extern int check_usermap(const char *usermap_name,
const char *pg_role, const char *auth_user,
