{"thread":{"id":"63592","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","startedAt":"2025-06-06T06:59:18Z","lastAt":"2025-06-06T20:08:22Z","messageCount":7,"participants":["Aditya Garg","Junio C Hamano"],"isPatch":true,"patchVersion":13,"patchTotal":10},"messages":[{"id":"519826","messageId":"PN3PR01MB95974F9ED808F60A915D054CB86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","threadId":"63592","inReplyTo":null,"subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Aditya Garg","fromEmail":"gargaditya08@live.com","sentAt":"2025-06-06T06:59:12Z","receivedAt":"2025-06-06T06:59:18Z","isPatch":true,"sender":{"key":"gargaditya08@live.com","avatar":"https://avatars.githubusercontent.com/u/85610623?v=4"},"body":"\n\n\n> On 6 Jun 2025, at 12:18 AM, Junio C Hamano wrote:\n> Aditya Garg writes:\n> \n>> Might look less ugly, but will result in a compiler warning that this will always\n>> be true if compiled with NO_OPENSSL. If you are fine with that, good. Else tbh\n>> I am out of ideas :(.\n> \n> Sounds like a good place to use NOT_CONSTANT(), it seems?\n\nOk so I was wrong here. The warning actually comes when we compile\nWITHOUT NO_OPENSSL, since auth_oauthbearer is a function there, and\nwill attract waddress warnings,\n\nAnd NOT_CONSTANT doesn't help here.\n> \n> if (NOT_CONSTANT(!auth_oauthbearer)) {\n> ... skip the thing ...\n> }\n> \n> \n>>>> server_fill_credential(srvc, cred);\n>>>> curl_easy_setopt(curl, CURLOPT_USERNAME, srvc->user);\n>>>> - curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n>>>> +\n>>>> + if (!srvc->auth_method ||\n>>>> + (strcmp(srvc->auth_method, \"XOAUTH2\") &&\n>>>> + strcmp(srvc->auth_method, \"OAUTHBEARER\")))\n>>>> + curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n>>> Can we clarify this part, possibly with an in-code comment?\n>>> \"Unless XOAUTH2 or OAUTHBEARER, use the password\" sounds a bit\n>>> strange. What about methods other than these two that are not a\n>>> plain simple password authentication? Will we remember extending\n>>> this code when we add yet another one to exclude it like XOAUTH2 and\n>>> OAUTHBEARER are excluded with this patch?\n> \n>> Let me answer this first. CURLOPT_PASSWORD is for plain or login type\n>> authentication, and if srvc->auth_method is not defined, curl's behaviour\n>> defaults to them.\n> \n> Which makes it sound like if (!srvc->auth_method) is enough?\n> \n>> OAUTHBEARER and XOAUTH2 use CURLOPT_XOAUTH2_BEARER\n>> in curl, which can use either of them based on what server says.\n> \n> That is what we can read from the updated code.\n> \n> The question is what happens when the user sets srvc->auth_method to\n> something other than NULL (unused---use plain password), \"XOAUTH2\"\n> or \"OAUTHBEARER\".\n> \n> If the answer to that question is ...\n> \n>> Other auth methods\n>> are not supported yet in this code, and this is the reason CRAM_MD5 is supported\n>> by only OpenSSL.\n> \n> ... \"with srvc->auth_method set to other methods like CRAM_MD5, the\n> control would never enter this codepath, as they are implemented\n> elsewhere\", then I think it would make more sense to write the above\n> like this:\n> \n> if (!srvc->auth_method)\n> curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n> else if (strcmp(srvc->auth_method, \"XOAUTH2\") &&\n> strcmp(srvc->auth_method, \"OAUTHBEARER\"))\n> BUG(\"we only support XOAUTH2 and OAUTHBEARER in this codepath\");\n> \n> Or the code is not protecting this code path so control can reach\n> with auth_method set to CRAM_MD5 here (e.g. when built without\n> OpenSSL)? If so, replace BUG(\"message\") with die(_(\"message\"))\n> above.\n> \n> On the other hand, if you are trying to fall back to plain password\n> when other unhandled methods are specified, I would expect that the\n> code to read more like:\n> \n> if (srvc->auth_method &&\n> (!strcmp(srvc->auth_method, \"XOAUTH2\") ||\n> !strcmp(srvc->auth_method, \"OAUTHBEARER\")))\n> ;\n> else {\n> if (srvc->auth_method)\n> warning(\"auth method %s not supported,\n> falling back to plain password\",\n> srvc->auth_method);\n> curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n> }\n> \n> I cannot quite tell which one you meant, but I am guessing that the\n> former is the case from your explanation.\n> \n> Thanks.\n"},{"id":"519827","messageId":"PN3PR01MB9597B34634910FE5E65ECBC6B86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","threadId":"63592","inReplyTo":"PN3PR01MB95974F9ED808F60A915D054CB86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Aditya Garg","fromEmail":"gargaditya08@live.com","sentAt":"2025-06-06T07:16:53Z","receivedAt":"2025-06-06T07:17:00Z","isPatch":true,"sender":{"key":"gargaditya08@live.com","avatar":"https://avatars.githubusercontent.com/u/85610623?v=4"},"body":"\n\n> On 6 Jun 2025, at 12:29 PM, Aditya Garg wrote:\n> \n> \n> \n> \n>> On 6 Jun 2025, at 12:18 AM, Junio C Hamano wrote:\n>> Aditya Garg writes:\n>> \n>>> Might look less ugly, but will result in a compiler warning that this will always\n>>> be true if compiled with NO_OPENSSL. If you are fine with that, good. Else tbh\n>>> I am out of ideas :(.\n>> \n>> Sounds like a good place to use NOT_CONSTANT(), it seems?\n> \n> Ok so I was wrong here. The warning actually comes when we compile\n> WITHOUT NO_OPENSSL, since auth_oauthbearer is a function there, and\n> will attract waddress warnings,\n> \n> And NOT_CONSTANT doesn't help here.\n\nSo this brings us back to the same problem. Compiled warnings (Waddress).\nShown without NO_OPENSSL and not with OPENSSL, with no other way in my\nmind than using macros for the compiler.\n\nOr maybe we can just use the original way, in which it died using a function?\n>> \n>> if (NOT_CONSTANT(!auth_oauthbearer)) {\n>> ... skip the thing ...\n>> }\n>> \n>> \n>>>>> server_fill_credential(srvc, cred);\n>>>>> curl_easy_setopt(curl, CURLOPT_USERNAME, srvc->user);\n>>>>> - curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n>>>>> +\n>>>>> + if (!srvc->auth_method ||\n>>>>> + (strcmp(srvc->auth_method, \"XOAUTH2\") &&\n>>>>> + strcmp(srvc->auth_method, \"OAUTHBEARER\")))\n>>>>> + curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n>>>> Can we clarify this part, possibly with an in-code comment?\n>>>> \"Unless XOAUTH2 or OAUTHBEARER, use the password\" sounds a bit\n>>>> strange. What about methods other than these two that are not a\n>>>> plain simple password authentication? Will we remember extending\n>>>> this code when we add yet another one to exclude it like XOAUTH2 and\n>>>> OAUTHBEARER are excluded with this patch?\n>> \n>>> Let me answer this first. CURLOPT_PASSWORD is for plain or login type\n>>> authentication, and if srvc->auth_method is not defined, curl's behaviour\n>>> defaults to them.\n>> \n>> Which makes it sound like if (!srvc->auth_method) is enough?\n>> \n>>> OAUTHBEARER and XOAUTH2 use CURLOPT_XOAUTH2_BEARER\n>>> in curl, which can use either of them based on what server says.\n>> \n>> That is what we can read from the updated code.\n>> \n>> The question is what happens when the user sets srvc->auth_method to\n>> something other than NULL (unused---use plain password), \"XOAUTH2\"\n>> or \"OAUTHBEARER\".\n>> \n>> If the answer to that question is ...\n>> \n>>> Other auth methods\n>>> are not supported yet in this code, and this is the reason CRAM_MD5 is supported\n>>> by only OpenSSL.\n>> \n>> ... \"with srvc->auth_method set to other methods like CRAM_MD5, the\n>> control would never enter this codepath, as they are implemented\n>> elsewhere\", then I think it would make more sense to write the above\n>> like this:\n>> \n>> if (!srvc->auth_method)\n>> curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n>> else if (strcmp(srvc->auth_method, \"XOAUTH2\") &&\n>> strcmp(srvc->auth_method, \"OAUTHBEARER\"))\n>> BUG(\"we only support XOAUTH2 and OAUTHBEARER in this codepath\");\n>> \n>> Or the code is not protecting this code path so control can reach\n>> with auth_method set to CRAM_MD5 here (e.g. when built without\n>> OpenSSL)? If so, replace BUG(\"message\") with die(_(\"message\"))\n>> above.\n>> \n>> On the other hand, if you are trying to fall back to plain password\n>> when other unhandled methods are specified, I would expect that the\n>> code to read more like:\n>> \n>> if (srvc->auth_method &&\n>> (!strcmp(srvc->auth_method, \"XOAUTH2\") ||\n>> !strcmp(srvc->auth_method, \"OAUTHBEARER\")))\n>> ;\n>> else {\n>> if (srvc->auth_method)\n>> warning(\"auth method %s not supported,\n>> falling back to plain password\",\n>> srvc->auth_method);\n>> curl_easy_setopt(curl, CURLOPT_PASSWORD, srvc->pass);\n>> }\n>> \n>> I cannot quite tell which one you meant, but I am guessing that the\n>> former is the case from your explanation.\n>> \n>> Thanks.\n"},{"id":"519858","messageId":"xmqqmsak3jwp.fsf@gitster.g","threadId":"63592","inReplyTo":"PN3PR01MB95974F9ED808F60A915D054CB86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-06T16:38:14Z","receivedAt":"2025-06-06T16:38:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aditya Garg writes:\n\n> Ok so I was wrong here. The warning actually comes when we compile\n> WITHOUT NO_OPENSSL, since auth_oauthbearer is a function there, and\n> will attract waddress warnings,\n\nBut the patch in this thread, when compiled with NO_OPENSSL, gives\n\"-Wunreachable\".\n\n--- * ------ * ------ * ------ * ------ * ---\n\n$ make V=1 CC=clang DEVELOPER=YesPlease CFLAGS=-Waddress NO_OPENSSL=NoThanks imap-send.o\n * new build flags\nclang -o imap-send.o -c -MF ./.depend/imap-send.o.d -MQ imap-send.o -MMD -MP -Werror -Wall -pedantic -Wpedantic -std=gnu99 -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -Wwrite-strings -fno-common -Wunreachable-code -Wcomma -Wtautological-constant-out-of-range-compare -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Waddress -I. -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DNO_OPENSSL -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"git-compat-util.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_SYSINFO -DHAVE_GETDELIM -DHAVE_GETRANDOM -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' imap-send.c\nimap-send.c:1344:5: error: code will never be executed [-Werror,-Wunreachable-code]\n 1344 | memset(&cb, 0, sizeof(cb));\n | ^~~~~~\nimap-send.c:1322:5: error: code will never be executed [-Werror,-Wunreachable-code]\n 1322 | memset(&cb, 0, sizeof(cb));\n | ^~~~~~\nimap-send.c:1300:5: error: code will never be executed [-Werror,-Wunreachable-code]\n 1300 | memset(&cb, 0, sizeof(cb));\n | ^~~~~~\nimap-send.c:1278:5: error: code will never be executed [-Werror,-Wunreachable-code]\n 1278 | memset(&cb, 0, sizeof(cb));\n | ^~~~~~\n4 errors generated.\nmake: *** [Makefile:2820: imap-send.o] Error 1\n\n--- * ------ * ------ * ------ * ------ * ---\n\nSee the patch attached at the end [*] that you can apply to 'seen'\nto try out; the result compiles with all combinations of gcc/clang\nand with and without NO_OPENSSL.\n\nHaving said that, I think you should first clean up the repetitions\nbefore piling a lot of different auth_method on top.\n\nA part of the resulting code after applying [*] to 'seen' looks like\nthis:\n\n\t} else if (!strcmp(srvc->auth_method, \"CRAM-MD5\")) {\n\t\tif (!CAP(AUTH_CRAM_MD5)) {\n\t\t\tfprintf(stderr, \"You specified \"\n\t\t\t\t\"CRAM-MD5 as authentication method, \"\n\t\t\t\t\"but %s doesn't support it.\\n\", srvc->host);\n\t\t\tgoto bail;\n\t\t}\n\n\t\t/* CRAM-MD5 */\n\t\tmemset(&cb, 0, sizeof(cb));\n\t\tcb.cont = auth_cram_md5;\n\n\t\tif (NOT_CONSTANT(!cb.cont)) {\n\t\tfprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n\t\t\t\"you have to build git-imap-send with OpenSSL library.\");\n\t\tgoto bail;\n\t\t}\n\t\tif (imap_exec(ctx, &cb, \"AUTHENTICATE CRAM-MD5\") != RESP_OK) {\n\t\t\tfprintf(stderr, \"IMAP error: AUTHENTICATE CRAM-MD5 failed\\n\");\n\t\t\tgoto bail;\n\t\t}\n\nThis pattern repeats for different auth methods as you add\ncopy-and-paste, which made the code way too long and too deeply\nindented to be readable. Why not make the above into a single\nhelper function that you can reuse, so that the call site can become\nsomething like\n\n\t} else if (!strcmp(srvc->auth_method, \"PLAIN\")) {\n\t if (try_auth_method(srvc, ctx, \"PLAIN\", AUTH_PLAIN, auth_plain))\n\t\tgoto bail;\n\t} else if (!strcmp(srvc->auth_method, \"CRAM-MD5\")) {\n\t if (try_auth_method(srvc, ctx, \"CRAM-MD5\", AUTH_CRAM_MD5, auth_cram_md5))\n\t\tgoto bail;\n\t} ...\n\nIf you did it at the very beginning of the series (i.e. when you\nonly have CRAM_MD5 you inherited from the original before adding all\nthe other new ones), each addition would become a lot simpler and\nmain flow in the the resulting code would become quite simple like\nthe above.\n\nstatic int try_auth_method(struct imap_server_conf *srvc,\n\t\t\t struct imap_store *ctx,\n\t\t\t const char *auth_method,\n\t\t\t enum CAPABILITY cap,\n\t\t\t int (*fn)(struct imap_store *, const char *))\n{\n struct imap_cmd_cb cb = {0};\n\n\tif (!CAP(cap)) {\n\t\tfprintf(stderr, \"You specified \"\n\t\t\t\"%s as authentication method, \"\n\t\t\t\"but %s doesn't support it.\\n\",\n\t\t\tauth_method, srvc->host);\n\t\treturn -1;\n\t}\n\tcb.cont = fn;\n\n\tif (NOT_CONSTANT(!cb.cont)) {\n\t\tfprintf(stderr, \"If you want to use %s authentication mechanism, \"\n\t\t\t\"you have to build git-imap-send with OpenSSL library.\",\n\t\t\tauth_method);\n\t\treturn -1;\n\t}\n\tif (imap_exec(ctx, &cb, \"AUTHENTICATE %s\", auth_method) != RESP_OK) {\n\t\tfprintf(stderr, \"IMAP error: AUTHENTICATE %s failed\\n\",\n \tauth_method);\n\t\treturn -1;\n\t}\n return 0;\n}\n\nSo here is that [*] patch that is incorrectly indented to avoid even\ndeeper nesting.\n\n---- >8 ----\n\n imap-send.c | 49 ++++++++++++++++++++++++-------------------------\n 1 file changed, 24 insertions(+), 25 deletions(-)\n\ndiff --git i/imap-send.c w/imap-send.c\nindex 6dbbf37125..bba2ef0cd0 100644\n--- i/imap-send.c\n+++ w/imap-send.c\n@@ -1267,16 +1267,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n \t\t\t\t\tgoto bail;\n \t\t\t\t}\n \n-\t\t\t\t#ifdef NO_OPENSSL\n+\t\t\t\t/* PLAIN */\n+\t\t\t\tmemset(&cb, 0, sizeof(cb));\n+\t\t\t\tcb.cont = auth_plain;\n+\n+\t\t\t\tif (NOT_CONSTANT(!cb.cont)) {\n \t\t\t\tfprintf(stderr, \"You are trying to use PLAIN authentication mechanism \"\n \t\t\t\t\t\"with OpenSSL library, but its support has not been compiled in.\");\n \t\t\t\tgoto bail;\n-\t\t\t\t#endif\n-\n-\t\t\t\t/* PLAIN */\n+\t\t\t\t}\n \n-\t\t\t\tmemset(&cb, 0, sizeof(cb));\n-\t\t\t\tcb.cont = auth_plain;\n \t\t\t\tif (imap_exec(ctx, &cb, \"AUTHENTICATE PLAIN\") != RESP_OK) {\n \t\t\t\t\tfprintf(stderr, \"IMAP error: AUTHENTICATE PLAIN failed\\n\");\n \t\t\t\t\tgoto bail;\n@@ -1289,16 +1289,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n \t\t\t\t\tgoto bail;\n \t\t\t\t}\n \n-\t\t\t\t#ifdef NO_OPENSSL\n-\t\t\t\tfprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n-\t\t\t\t\t\"you have to build git-imap-send with OpenSSL library.\");\n-\t\t\t\tgoto bail;\n-\t\t\t\t#endif\n \n \t\t\t\t/* CRAM-MD5 */\n-\n \t\t\t\tmemset(&cb, 0, sizeof(cb));\n \t\t\t\tcb.cont = auth_cram_md5;\n+\n+\t\t\t\tif (NOT_CONSTANT(!cb.cont)) {\n+\t\t\t\tfprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n+\t\t\t\t\t\"you have to build git-imap-send with OpenSSL library.\");\n+\t\t\t\tgoto bail;\n+\t\t\t\t}\n \t\t\t\tif (imap_exec(ctx, &cb, \"AUTHENTICATE CRAM-MD5\") != RESP_OK) {\n \t\t\t\t\tfprintf(stderr, \"IMAP error: AUTHENTICATE CRAM-MD5 failed\\n\");\n \t\t\t\t\tgoto bail;\n@@ -1311,16 +1311,15 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n \t\t\t\t\tgoto bail;\n \t\t\t\t}\n \n-\t\t\t\t#ifdef NO_OPENSSL\n-\t\t\t\tfprintf(stderr, \"You are trying to use OAUTHBEARER authentication mechanism \"\n-\t\t\t\t\t\"with OpenSSL library, but its support has not been compiled in.\");\n-\t\t\t\tgoto bail;\n-\t\t\t\t#endif\n-\n \t\t\t\t/* OAUTHBEARER */\n-\n \t\t\t\tmemset(&cb, 0, sizeof(cb));\n \t\t\t\tcb.cont = auth_oauthbearer;\n+\n+\t\t\t\tif (NOT_CONSTANT(!cb.cont)) {\n+\t\t\t\tfprintf(stderr, \"You are trying to use OAUTHBEARER authentication mechanism \"\n+\t\t\t\t\t\"with OpenSSL library, but its support has not been compiled in.\");\n+\t\t\t\tgoto bail;\n+\t\t\t\t}\n \t\t\t\tif (imap_exec(ctx, &cb, \"AUTHENTICATE OAUTHBEARER\") != RESP_OK) {\n \t\t\t\t\tfprintf(stderr, \"IMAP error: AUTHENTICATE OAUTHBEARER failed\\n\");\n \t\t\t\t\tgoto bail;\n@@ -1333,16 +1332,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n \t\t\t\t\tgoto bail;\n \t\t\t\t}\n \n-\t\t\t\t#ifdef NO_OPENSSL\n+\t\t\t\t/* XOAUTH2 */\n+\t\t\t\tmemset(&cb, 0, sizeof(cb));\n+\t\t\t\tcb.cont = auth_xoauth2;\n+\n+\t\t\t\tif (NOT_CONSTANT(!cb.cont)) {\n \t\t\t\tfprintf(stderr, \"You are trying to use XOAUTH2 authentication mechanism \"\n \t\t\t\t\t\"with OpenSSL library, but its support has not been compiled in.\");\n \t\t\t\tgoto bail;\n-\t\t\t\t#endif\n-\n-\t\t\t\t/* XOAUTH2 */\n+\t\t\t\t}\n \n-\t\t\t\tmemset(&cb, 0, sizeof(cb));\n-\t\t\t\tcb.cont = auth_xoauth2;\n \t\t\t\tif (imap_exec(ctx, &cb, \"AUTHENTICATE XOAUTH2\") != RESP_OK) {\n \t\t\t\t\tfprintf(stderr, \"IMAP error: AUTHENTICATE XOAUTH2 failed\\n\");\n \t\t\t\t\tgoto bail;\n"},{"id":"519860","messageId":"PN3PR01MB959762A91208AEF089D3302BB86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","threadId":"63592","inReplyTo":"xmqqmsak3jwp.fsf@gitster.g","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Aditya Garg","fromEmail":"gargaditya08@live.com","sentAt":"2025-06-06T16:50:49Z","receivedAt":"2025-06-06T16:50:54Z","isPatch":true,"sender":{"key":"gargaditya08@live.com","avatar":"https://avatars.githubusercontent.com/u/85610623?v=4"},"body":"\n\n> On 6 Jun 2025, at 10:08 PM, Junio C Hamano wrote:\n> \n> Aditya Garg writes:\n> \n>> Ok so I was wrong here. The warning actually comes when we compile\n>> WITHOUT NO_OPENSSL, since auth_oauthbearer is a function there, and\n>> will attract waddress warnings,\n> \n> But the patch in this thread, when compiled with NO_OPENSSL, gives\n> \"-Wunreachable\".\n\nUggh\n> \n> --- * ------ * ------ * ------ * ------ * ---\n> \n> $ make V=1 CC=clang DEVELOPER=YesPlease CFLAGS=-Waddress NO_OPENSSL=NoThanks imap-send.o\n> * new build flags\n> clang -o imap-send.o -c -MF ./.depend/imap-send.o.d -MQ imap-send.o -MMD -MP -Werror -Wall -pedantic -Wpedantic -std=gnu99 -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -Wwrite-strings -fno-common -Wunreachable-code -Wcomma -Wtautological-constant-out-of-range-compare -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Waddress -I. -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DNO_OPENSSL -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"git-compat-util.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_SYSINFO -DHAVE_GETDELIM -DHAVE_GETRANDOM -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' imap-send.c\n> imap-send.c:1344:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1344 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> imap-send.c:1322:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1322 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> imap-send.c:1300:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1300 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> imap-send.c:1278:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1278 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> 4 errors generated.\n> make: *** [Makefile:2820: imap-send.o] Error 1\n> \n> --- * ------ * ------ * ------ * ------ * ---\n> \n> See the patch attached at the end [*] that you can apply to 'seen'\n> to try out; the result compiles with all combinations of gcc/clang\n> and with and without NO_OPENSSL.\n\nGreat, thanks for the patch. Lemme see if I can make something nice.\n> \n> Having said that, I think you should first clean up the repetitions\n> before piling a lot of different auth_method on top.\n> \n> A part of the resulting code after applying [*] to 'seen' looks like\n> this:\n> \n> } else if (!strcmp(srvc->auth_method, \"CRAM-MD5\")) {\n> if (!CAP(AUTH_CRAM_MD5)) {\n> fprintf(stderr, \"You specified \"\n> \"CRAM-MD5 as authentication method, \"\n> \"but %s doesn't support it.\\n\", srvc->host);\n> goto bail;\n> }\n> \n> /* CRAM-MD5 */\n> memset(&cb, 0, sizeof(cb));\n> cb.cont = auth_cram_md5;\n> \n> if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n> \"you have to build git-imap-send with OpenSSL library.\");\n> goto bail;\n> }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE CRAM-MD5\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE CRAM-MD5 failed\\n\");\n> goto bail;\n> }\n> \n> This pattern repeats for different auth methods as you add\n> copy-and-paste, which made the code way too long and too deeply\n> indented to be readable. Why not make the above into a single\n> helper function that you can reuse, so that the call site can become\n> something like\n\nI was thinking the same but though the code would be more readable\nin case if repeats. With GCC, even the binaries were of the same size.\n> \n> } else if (!strcmp(srvc->auth_method, \"PLAIN\")) {\n> if (try_auth_method(srvc, ctx, \"PLAIN\", AUTH_PLAIN, auth_plain))\n> goto bail;\n> } else if (!strcmp(srvc->auth_method, \"CRAM-MD5\")) {\n> if (try_auth_method(srvc, ctx, \"CRAM-MD5\", AUTH_CRAM_MD5, auth_cram_md5))\n> goto bail;\n> } ...\n> \n> If you did it at the very beginning of the series (i.e. when you\n> only have CRAM_MD5 you inherited from the original before adding all\n> the other new ones), each addition would become a lot simpler and\n> main flow in the the resulting code would become quite simple like\n> the above.\n> \n> static int try_auth_method(struct imap_server_conf *srvc,\n> struct imap_store *ctx,\n> const char *auth_method,\n> enum CAPABILITY cap,\n> int (*fn)(struct imap_store *, const char *))\n> {\n> struct imap_cmd_cb cb = {0};\n> \n> if (!CAP(cap)) {\n> fprintf(stderr, \"You specified \"\n> \"%s as authentication method, \"\n> \"but %s doesn't support it.\\n\",\n> auth_method, srvc->host);\n> return -1;\n> }\n> cb.cont = fn;\n> \n> if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"If you want to use %s authentication mechanism, \"\n> \"you have to build git-imap-send with OpenSSL library.\",\n> auth_method);\n> return -1;\n> }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE %s\", auth_method) != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE %s failed\\n\",\n> auth_method);\n> return -1;\n> }\n> return 0;\n> }\n> \n> So here is that [*] patch that is incorrectly indented to avoid even\n> deeper nesting.\n> \n> ---- >8 ----\n> \n> imap-send.c | 49 ++++++++++++++++++++++++-------------------------\n> 1 file changed, 24 insertions(+), 25 deletions(-)\n> \n> diff --git i/imap-send.c w/imap-send.c\n> index 6dbbf37125..bba2ef0cd0 100644\n> --- i/imap-send.c\n> +++ w/imap-send.c\n> @@ -1267,16 +1267,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> + /* PLAIN */\n> + memset(&cb, 0, sizeof(cb));\n> + cb.cont = auth_plain;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"You are trying to use PLAIN authentication mechanism \"\n> \"with OpenSSL library, but its support has not been compiled in.\");\n> goto bail;\n> - #endif\n> -\n> - /* PLAIN */\n> + }\n> \n> - memset(&cb, 0, sizeof(cb));\n> - cb.cont = auth_plain;\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE PLAIN\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE PLAIN failed\\n\");\n> goto bail;\n> @@ -1289,16 +1289,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> - fprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n> - \"you have to build git-imap-send with OpenSSL library.\");\n> - goto bail;\n> - #endif\n> \n> /* CRAM-MD5 */\n> -\n> memset(&cb, 0, sizeof(cb));\n> cb.cont = auth_cram_md5;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> + fprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n> + \"you have to build git-imap-send with OpenSSL library.\");\n> + goto bail;\n> + }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE CRAM-MD5\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE CRAM-MD5 failed\\n\");\n> goto bail;\n> @@ -1311,16 +1311,15 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> - fprintf(stderr, \"You are trying to use OAUTHBEARER authentication mechanism \"\n> - \"with OpenSSL library, but its support has not been compiled in.\");\n> - goto bail;\n> - #endif\n> -\n> /* OAUTHBEARER */\n> -\n> memset(&cb, 0, sizeof(cb));\n> cb.cont = auth_oauthbearer;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> + fprintf(stderr, \"You are trying to use OAUTHBEARER authentication mechanism \"\n> + \"with OpenSSL library, but its support has not been compiled in.\");\n> + goto bail;\n> + }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE OAUTHBEARER\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE OAUTHBEARER failed\\n\");\n> goto bail;\n> @@ -1333,16 +1332,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> + /* XOAUTH2 */\n> + memset(&cb, 0, sizeof(cb));\n> + cb.cont = auth_xoauth2;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"You are trying to use XOAUTH2 authentication mechanism \"\n> \"with OpenSSL library, but its support has not been compiled in.\");\n> goto bail;\n> - #endif\n> -\n> - /* XOAUTH2 */\n> + }\n> \n> - memset(&cb, 0, sizeof(cb));\n> - cb.cont = auth_xoauth2;\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE XOAUTH2\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE XOAUTH2 failed\\n\");\n> goto bail;\n"},{"id":"519862","messageId":"xmqqbjr03ipu.fsf@gitster.g","threadId":"63592","inReplyTo":"PN3PR01MB959762A91208AEF089D3302BB86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-06T17:03:57Z","receivedAt":"2025-06-06T17:04:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aditya Garg writes:\n\n> I was thinking the same but though the code would be more readable\n> in case if repeats.\n\nSorry but I do not understand this comment at all. Not repeating so\nthat you do not have to fix or update all the repetitions later when\nthe code needs to do something different is one of the basics in the\nsoftware engineering, and less repetitious code is also easier to\nunderstnad in the art of software field.\n\nIf you start from a 20-line block and repeat it 5 times to grow it\n100 lines of code, the eyes of readers will start coasting over\nafter a few repetitions, and you may be able to smuggle unwanted\nlines unnoticed. Unless you are aiming for such technique to do\nsomething malicious, don't go there.\n"},{"id":"519863","messageId":"PN3PR01MB959793AA498C4FD81313B101B86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","threadId":"63592","inReplyTo":"xmqqbjr03ipu.fsf@gitster.g","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Aditya Garg","fromEmail":"gargaditya08@live.com","sentAt":"2025-06-06T17:12:36Z","receivedAt":"2025-06-06T17:12:41Z","isPatch":true,"sender":{"key":"gargaditya08@live.com","avatar":"https://avatars.githubusercontent.com/u/85610623?v=4"},"body":"\n\n> On 6 Jun 2025, at 10:34 PM, Junio C Hamano wrote:\n> \n> Aditya Garg writes:\n> \n>> I was thinking the same but though the code would be more readable\n>> in case if repeats.\n> \n> Sorry but I do not understand this comment at all. Not repeating so\n> that you do not have to fix or update all the repetitions later when\n> the code needs to do something different is one of the basics in the\n> software engineering, and less repetitious code is also easier to\n> understnad in the art of software field.\n> \n> If you start from a 20-line block and repeat it 5 times to grow it\n> 100 lines of code, the eyes of readers will start coasting over\n> after a few repetitions, and you may be able to smuggle unwanted\n> lines unnoticed. Unless you are aiming for such technique to do\n> something malicious, don't go there.\n\nOMG why would I be malicious (˃̣̣̥ᯅ˂̣̣̥) It's just a matter of my personal taste.\nObviously your points are valid. And sorry for my ignorance, I am still\na rookie dev."},{"id":"519864","messageId":"PN3PR01MB9597A2AA9D01809A55FCA5B5B86EA@PN3PR01MB9597.INDPRD01.PROD.OUTLOOK.COM","threadId":"63592","inReplyTo":"xmqqmsak3jwp.fsf@gitster.g","subject":"Re: [PATCH v13 02/10] imap-send: add support for OAuth2.0 authentication","fromName":"Aditya Garg","fromEmail":"gargaditya08@live.com","sentAt":"2025-06-06T20:08:17Z","receivedAt":"2025-06-06T20:08:22Z","isPatch":true,"sender":{"key":"gargaditya08@live.com","avatar":"https://avatars.githubusercontent.com/u/85610623?v=4"},"body":"\n\n> On 6 Jun 2025, at 10:08 PM, Junio C Hamano wrote:\n> \n> Aditya Garg writes:\n> \n>> Ok so I was wrong here. The warning actually comes when we compile\n>> WITHOUT NO_OPENSSL, since auth_oauthbearer is a function there, and\n>> will attract waddress warnings,\n> \n> But the patch in this thread, when compiled with NO_OPENSSL, gives\n> \"-Wunreachable\".\n> \n> --- * ------ * ------ * ------ * ------ * ---\n> \n> $ make V=1 CC=clang DEVELOPER=YesPlease CFLAGS=-Waddress NO_OPENSSL=NoThanks imap-send.o\n> * new build flags\n> clang -o imap-send.o -c -MF ./.depend/imap-send.o.d -MQ imap-send.o -MMD -MP -Werror -Wall -pedantic -Wpedantic -std=gnu99 -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -Wwrite-strings -fno-common -Wunreachable-code -Wcomma -Wtautological-constant-out-of-range-compare -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Waddress -I. -DGIT_HOST_CPU=\"\\\"x86_64\\\"\" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DNO_OPENSSL -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C=\"\\\"git-compat-util.h\\\"\" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C=\"\\\"git-compat-util.h\\\"\" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_SYSINFO -DHAVE_GETDELIM -DHAVE_GETRANDOM -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='\"/bin/sh\"' imap-send.c\n> imap-send.c:1344:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1344 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> imap-send.c:1322:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1322 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> imap-send.c:1300:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1300 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> imap-send.c:1278:5: error: code will never be executed [-Werror,-Wunreachable-code]\n> 1278 | memset(&cb, 0, sizeof(cb));\n> | ^~~~~~\n> 4 errors generated.\n> make: *** [Makefile:2820: imap-send.o] Error 1\n> \n> --- * ------ * ------ * ------ * ------ * ---\n> \n> See the patch attached at the end [*] that you can apply to 'seen'\n> to try out; the result compiles with all combinations of gcc/clang\n> and with and without NO_OPENSSL.\n> \n> Having said that, I think you should first clean up the repetitions\n> before piling a lot of different auth_method on top.\n> \n> A part of the resulting code after applying [*] to 'seen' looks like\n> this:\n> \n> } else if (!strcmp(srvc->auth_method, \"CRAM-MD5\")) {\n> if (!CAP(AUTH_CRAM_MD5)) {\n> fprintf(stderr, \"You specified \"\n> \"CRAM-MD5 as authentication method, \"\n> \"but %s doesn't support it.\\n\", srvc->host);\n> goto bail;\n> }\n> \n> /* CRAM-MD5 */\n> memset(&cb, 0, sizeof(cb));\n> cb.cont = auth_cram_md5;\n> \n> if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n> \"you have to build git-imap-send with OpenSSL library.\");\n> goto bail;\n> }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE CRAM-MD5\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE CRAM-MD5 failed\\n\");\n> goto bail;\n> }\n> \n> This pattern repeats for different auth methods as you add\n> copy-and-paste, which made the code way too long and too deeply\n> indented to be readable. Why not make the above into a single\n> helper function that you can reuse, so that the call site can become\n> something like\n> \n> } else if (!strcmp(srvc->auth_method, \"PLAIN\")) {\n> if (try_auth_method(srvc, ctx, \"PLAIN\", AUTH_PLAIN, auth_plain))\n> goto bail;\n> } else if (!strcmp(srvc->auth_method, \"CRAM-MD5\")) {\n> if (try_auth_method(srvc, ctx, \"CRAM-MD5\", AUTH_CRAM_MD5, auth_cram_md5))\n> goto bail;\n> } ...\n> \n> If you did it at the very beginning of the series (i.e. when you\n> only have CRAM_MD5 you inherited from the original before adding all\n> the other new ones), each addition would become a lot simpler and\n> main flow in the the resulting code would become quite simple like\n> the above.\n> \n> static int try_auth_method(struct imap_server_conf *srvc,\n> struct imap_store *ctx,\nHad to add struct imap *imap, as well here. With that added, a v14 has been sent.\n> const char *auth_method,\n> enum CAPABILITY cap,\n> int (*fn)(struct imap_store *, const char *))\n> {\n> struct imap_cmd_cb cb = {0};\n> \n> if (!CAP(cap)) {\n> fprintf(stderr, \"You specified \"\n> \"%s as authentication method, \"\n> \"but %s doesn't support it.\\n\",\n> auth_method, srvc->host);\n> return -1;\n> }\n> cb.cont = fn;\n> \n> if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"If you want to use %s authentication mechanism, \"\n> \"you have to build git-imap-send with OpenSSL library.\",\n> auth_method);\n> return -1;\n> }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE %s\", auth_method) != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE %s failed\\n\",\n> auth_method);\n> return -1;\n> }\n> return 0;\n> }\n> \n> So here is that [*] patch that is incorrectly indented to avoid even\n> deeper nesting.\n> \n> ---- >8 ----\n> \n> imap-send.c | 49 ++++++++++++++++++++++++-------------------------\n> 1 file changed, 24 insertions(+), 25 deletions(-)\n> \n> diff --git i/imap-send.c w/imap-send.c\n> index 6dbbf37125..bba2ef0cd0 100644\n> --- i/imap-send.c\n> +++ w/imap-send.c\n> @@ -1267,16 +1267,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> + /* PLAIN */\n> + memset(&cb, 0, sizeof(cb));\n> + cb.cont = auth_plain;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"You are trying to use PLAIN authentication mechanism \"\n> \"with OpenSSL library, but its support has not been compiled in.\");\n> goto bail;\n> - #endif\n> -\n> - /* PLAIN */\n> + }\n> \n> - memset(&cb, 0, sizeof(cb));\n> - cb.cont = auth_plain;\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE PLAIN\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE PLAIN failed\\n\");\n> goto bail;\n> @@ -1289,16 +1289,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> - fprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n> - \"you have to build git-imap-send with OpenSSL library.\");\n> - goto bail;\n> - #endif\n> \n> /* CRAM-MD5 */\n> -\n> memset(&cb, 0, sizeof(cb));\n> cb.cont = auth_cram_md5;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> + fprintf(stderr, \"If you want to use CRAM-MD5 authentication mechanism, \"\n> + \"you have to build git-imap-send with OpenSSL library.\");\n> + goto bail;\n> + }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE CRAM-MD5\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE CRAM-MD5 failed\\n\");\n> goto bail;\n> @@ -1311,16 +1311,15 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> - fprintf(stderr, \"You are trying to use OAUTHBEARER authentication mechanism \"\n> - \"with OpenSSL library, but its support has not been compiled in.\");\n> - goto bail;\n> - #endif\n> -\n> /* OAUTHBEARER */\n> -\n> memset(&cb, 0, sizeof(cb));\n> cb.cont = auth_oauthbearer;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> + fprintf(stderr, \"You are trying to use OAUTHBEARER authentication mechanism \"\n> + \"with OpenSSL library, but its support has not been compiled in.\");\n> + goto bail;\n> + }\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE OAUTHBEARER\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE OAUTHBEARER failed\\n\");\n> goto bail;\n> @@ -1333,16 +1332,16 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c\n> goto bail;\n> }\n> \n> - #ifdef NO_OPENSSL\n> + /* XOAUTH2 */\n> + memset(&cb, 0, sizeof(cb));\n> + cb.cont = auth_xoauth2;\n> +\n> + if (NOT_CONSTANT(!cb.cont)) {\n> fprintf(stderr, \"You are trying to use XOAUTH2 authentication mechanism \"\n> \"with OpenSSL library, but its support has not been compiled in.\");\n> goto bail;\n> - #endif\n> -\n> - /* XOAUTH2 */\n> + }\n> \n> - memset(&cb, 0, sizeof(cb));\n> - cb.cont = auth_xoauth2;\n> if (imap_exec(ctx, &cb, \"AUTHENTICATE XOAUTH2\") != RESP_OK) {\n> fprintf(stderr, \"IMAP error: AUTHENTICATE XOAUTH2 failed\\n\");\n> goto bail;\n"}]}