Permission: refactor getPermissionFromPassword() to return std::optional

This replaces the output parameter (which is bad API design). As a side effect, it fixes the bad [[gnu::pure]] attribute added by commit a636d212 which caused optimizing compilers to miscompile calls to that function. "Pure" functions can be assumed to have no output arguments, so the compiler can assume the function doesn't modify them. Closes https://github.com/MusicPlayerDaemon/MPD/issues/1282

Permission: refactor getPermissionFromPassword() to return std::optional
907af2ad · Max Kellermann · 6a2e7bbc · 907af2ad · 907af2ad · 907af2ad
Commit 907af2ad authored Oct 17, 2021 by Max Kellermann
Hide whitespace changes
Inline Side-by-side

Showing with 16 additions and 10 deletions

NEWS NEWS +1 -0

Permission.cxx src/Permission.cxx +4 -5

Permission.hxx src/Permission.hxx +8 -2

ClientCommands.cxx src/command/ClientCommands.cxx +3 -3

No files found.
--- a/NEWS
+++ b/NEWS
 ver 0.23.1 (not yet released)
 * fix libfmt linker problems
+* fix broken password authentication

 ver 0.23 (2021/10/14)
 * protocol

--- a/src/Permission.cxx
+++ b/src/Permission.cxx
@@ -161,15 +161,14 @@ GetPermissionsFromAddress(SocketAddress address) noexcept

 #endif

-int
-getPermissionFromPassword(const char *password, unsigned *permission) noexcept
+std::optional<unsigned>
+GetPermissionFromPassword(const char *password) noexcept
 {
 	auto i = permission_passwords.find(password);
 	if (i == permission_passwords.end())
-		return -1;
+		return std::nullopt;

-	*permission = i->second;
-	return 0;
+	return i->second;
 }

 unsigned

--- a/src/Permission.hxx
+++ b/src/Permission.hxx
@@ -22,6 +22,8 @@

 #include "config.h"

+#include <optional>
+
 struct ConfigData;
 class SocketAddress;

@@ -32,9 +34,13 @@ static constexpr unsigned PERMISSION_CONTROL = 4;
 static constexpr unsigned PERMISSION_ADMIN = 8;
 static constexpr unsigned PERMISSION_PLAYER = 16;

+/**
+ * @return the permissions for the given password or std::nullopt if
+ * the password is not accepted
+ */
 [[gnu::pure]]
-int
-getPermissionFromPassword(const char *password, unsigned *permission) noexcept;
+std::optional<unsigned>
+GetPermissionFromPassword(const char *password) noexcept;

 [[gnu::const]]
 unsigned

--- a/src/command/ClientCommands.cxx
+++ b/src/command/ClientCommands.cxx
@@ -58,13 +58,13 @@ handle_binary_limit(Client &client, Request args,
 CommandResult
 handle_password(Client &client, Request args, Response &r)
 {
-	unsigned permission = 0;
-	if (getPermissionFromPassword(args.front(), &permission) < 0) {
+	const auto permission = GetPermissionFromPassword(args.front());
+	if (!permission) {
 		r.Error(ACK_ERROR_PASSWORD, "incorrect password");
 		return CommandResult::ERROR;
 	}

-	client.SetPermission(permission);
+	client.SetPermission(*permission);

 	return CommandResult::OK;
 }