From 907af2ad02aa8a7b0a5c5efedda27960be989868 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Sun, 17 Oct 2021 19:50:59 +0200
Subject: [PATCH] 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
a636d2127 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
---
 NEWS                           |  1 +
 src/Permission.cxx             |  9 ++++-----
 src/Permission.hxx             | 10 ++++++++--
 src/command/ClientCommands.cxx |  6 +++---
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 4cd880a0a..a4492a67b 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
 ver 0.23.1 (not yet released)
 * fix libfmt linker problems
+* fix broken password authentication
 
 ver 0.23 (2021/10/14)
 * protocol
diff --git a/src/Permission.cxx b/src/Permission.cxx
index 6343f2d50..2ba56c8eb 100644
--- 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
diff --git a/src/Permission.hxx b/src/Permission.hxx
index 2d9dfb9ae..8304fa26e 100644
--- 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
diff --git a/src/command/ClientCommands.cxx b/src/command/ClientCommands.cxx
index 01bd462f2..7d2148106 100644
--- 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;
 }