[v2,2/2] osxkeychain: state[] seen=1 to skip unnecessary store operations

Commit Message

Koji Nakamaru May 11, 2024, 11:55 a.m. UTC

From: Koji Nakamaru <[e-mail]>

Records whether credentials come from get operations and skips
unnecessary store operations by utilizing the state[] feature, as
suggested by brian m. carlson.

Signed-off-by: Koji Nakamaru <[e-mail]>
---
 .../osxkeychain/git-credential-osxkeychain.c          | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Junio C Hamano May 12, 2024, 4:09 a.m. UTC | #1

"Koji Nakamaru via GitGitGadget" <[e-mail]> writes:

> From: Koji Nakamaru <[e-mail]>
>
> Records whether credentials come from get operations and skips
> unnecessary store operations by utilizing the state[] feature, as
> suggested by brian m. carlson.

This step has a problem description that is even sketchier than the
previous one.  Anticipate questions by the other developers who read
this commit 6 months after it is accepted in the mainline (e.g.,
What problem is there in the current system, why it is bad and worth
solving, and how is the patch trying to solve it?) and let your
proposed log message answer the questions, as you won't be always
sitting next to these developers.

Thanks.

Koji Nakamaru May 12, 2024, 7:05 a.m. UTC | #2

Thank you. I'll clean up the whole patch and its description after
getting a final reply about exlock.

Koji Nakamaru

Patch

diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index 0884db48d0a..6ce22a28ed7 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -12,6 +12,7 @@  static CFStringRef username;
 static CFDataRef password;
 static CFDataRef password_expiry_utc;
 static CFDataRef oauth_refresh_token;
+static int state_seen;
 
 static void clear_credential(void)
 {
@@ -171,6 +172,9 @@  static OSStatus find_internet_password(void)
 
 	CFRelease(item);
 
+	write_item("capability[]", "state", strlen("state"));
+	write_item("state[]", "osxkeychain:seen=1", strlen("osxkeychain:seen=1"));
+
 out:
 	CFRelease(attrs);
 
@@ -284,6 +288,9 @@  static OSStatus add_internet_password(void)
 	CFDictionaryRef attrs;
 	OSStatus result;
 
+	if (state_seen)
+		return errSecSuccess;
+
 	/* Only store complete credentials */
 	if (!protocol || !host || !username || !password)
 		return -1;
@@ -395,6 +402,10 @@  static void read_credential(void)
 			oauth_refresh_token = CFDataCreate(kCFAllocatorDefault,
 							   (UInt8 *)v,
 							   strlen(v));
+		else if (!strcmp(buf, "state[]")) {
+			if (!strcmp(v, "osxkeychain:seen=1"))
+				state_seen = 1;
+		}
 		/*
 		 * Ignore other lines; we don't know what they mean, but
 		 * this future-proofs us when later versions of git do