{"thread":{"id":"36319","subject":"socket_perror() \"bug\"?","startedAt":"2014-03-30T19:32:54Z","lastAt":"2014-04-03T19:01:32Z","messageCount":4,"participants":["Thiago Farina","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"238104","messageId":"CACnwZYc2py4dxehg2=gnnPLxwJaRqXYTLQvC1O7YuoqAWsZ0Tg@mail.gmail.com","threadId":"36319","inReplyTo":null,"subject":"socket_perror() \"bug\"?","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2014-03-30T19:32:54Z","receivedAt":"2014-03-30T19:32:54Z","isPatch":false,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"Hi,\n\nIn imap-send.c:socket_perror() we pass |func| as a parameter, which I\nthink it is the name of the function that \"called\" socket_perror, or\nthe name of the function which generated an error.\n\nBut at line 184 and 187 it always assume it was SSL_connect.\n\nShould we instead call perror() and ssl_socket_error() with func?\n\n--\nThiago Farina\n"},{"id":"238144","messageId":"xmqqy4zq3xek.fsf@gitster.dls.corp.google.com","threadId":"36319","inReplyTo":"CACnwZYc2py4dxehg2=gnnPLxwJaRqXYTLQvC1O7YuoqAWsZ0Tg@mail.gmail.com","subject":"Re: socket_perror() \"bug\"?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-31T20:50:43Z","receivedAt":"2014-03-31T20:50:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina <tfransosi@gmail.com> writes:\n\n> In imap-send.c:socket_perror() we pass |func| as a parameter, which I\n> think it is the name of the function that \"called\" socket_perror, or\n> the name of the function which generated an error.\n>\n> But at line 184 and 187 it always assume it was SSL_connect.\n>\n> Should we instead call perror() and ssl_socket_error() with func?\n\nLooks that way to me, at least from a cursory look.\n"},{"id":"238309","messageId":"CACnwZYf30KLVLkaB4mNrW12DHwrf=RT7H-DBNvQYs0y6RqVGLw@mail.gmail.com","threadId":"36319","inReplyTo":"xmqqy4zq3xek.fsf@gitster.dls.corp.google.com","subject":"Re: socket_perror() \"bug\"?","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2014-04-02T23:05:01Z","receivedAt":"2014-04-02T23:05:01Z","isPatch":false,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Mon, Mar 31, 2014 at 5:50 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Thiago Farina <tfransosi@gmail.com> writes:\n>\n>> In imap-send.c:socket_perror() we pass |func| as a parameter, which I\n>> think it is the name of the function that \"called\" socket_perror, or\n>> the name of the function which generated an error.\n>>\n>> But at line 184 and 187 it always assume it was SSL_connect.\n>>\n>> Should we instead call perror() and ssl_socket_error() with func?\n>\n> Looks that way to me, at least from a cursory look.\nWould you accept such a patch?\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 0bc6f7f..bb04768 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -181,10 +181,10 @@ static void socket_perror(const char *func,\nstruct imap_socket *sock, int ret)\n                case SSL_ERROR_NONE:\n                        break;\n                case SSL_ERROR_SYSCALL:\n-                       perror(\"SSL_connect\");\n+                       perror(func);\n                        break;\n                default:\n-                       ssl_socket_perror(\"SSL_connect\");\n+                       ssl_socket_perror(func);\n                        break;\n                }\n        } else\n\n--\nThiago Farina\n"},{"id":"238362","messageId":"xmqq1txeqltf.fsf@gitster.dls.corp.google.com","threadId":"36319","inReplyTo":"CACnwZYf30KLVLkaB4mNrW12DHwrf=RT7H-DBNvQYs0y6RqVGLw@mail.gmail.com","subject":"Re: socket_perror() \"bug\"?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-03T19:01:32Z","receivedAt":"2014-04-03T19:01:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina <tfransosi@gmail.com> writes:\n\n> On Mon, Mar 31, 2014 at 5:50 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Thiago Farina <tfransosi@gmail.com> writes:\n>>\n>>> In imap-send.c:socket_perror() we pass |func| as a parameter, which I\n>>> think it is the name of the function that \"called\" socket_perror, or\n>>> the name of the function which generated an error.\n>>>\n>>> But at line 184 and 187 it always assume it was SSL_connect.\n>>>\n>>> Should we instead call perror() and ssl_socket_error() with func?\n>>\n>> Looks that way to me, at least from a cursory look.\n> Would you accept such a patch?\n\nThis back-and-forth makes me wonder what is going on.  Why not send\na full patch with a proper proposed commit log message to the list\nand see what happens?\n\n> diff --git a/imap-send.c b/imap-send.c\n> index 0bc6f7f..bb04768 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -181,10 +181,10 @@ static void socket_perror(const char *func,\n> struct imap_socket *sock, int ret)\n>                 case SSL_ERROR_NONE:\n>                         break;\n>                 case SSL_ERROR_SYSCALL:\n> -                       perror(\"SSL_connect\");\n> +                       perror(func);\n>                         break;\n>                 default:\n> -                       ssl_socket_perror(\"SSL_connect\");\n> +                       ssl_socket_perror(func);\n>                         break;\n>                 }\n>         } else\n>\n> --\n> Thiago Farina\n"}]}