[PATCH] imap-send.c: fix pointer to be const

[PATCH] imap-send.c: fix pointer to be const

[Vietor Liu]

Fixes some compiler warnings:
imap-send.c: In function ‘ssl_socket_connect’:
warning: assignment discards qualifiers from pointer target type

Signed-off-by: Vietor Liu <vietor@vxwo.org>
---
 imap-send.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 3847fd1..10dd025 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
 	return -1;
 #else
-	SSL_METHOD *meth;
+	const SSL_METHOD *meth;
 	SSL_CTX *ctx;
 	int ret;
 
-- 
1.6.5.2

Re: [PATCH] imap-send.c: fix pointer to be const

[Junio C Hamano]

Vietor Liu <vietor@vxwo.org> writes:

> Fixes some compiler warnings:
> imap-send.c: In function ‘ssl_socket_connect’:
> warning: assignment discards qualifiers from pointer target type
>
> Signed-off-by: Vietor Liu <vietor@vxwo.org>

I do not quite understand.  This variable gets assigned the return values
from TLSv1_method() or SSLv23_method(), but the copy of ssl.h I have
declares them as:

    SSL_METHOD *SSLv23_method(void);	/* SSLv3 but can rollback to v2 */
    SSL_METHOD *TLSv1_method(void);		/* TLSv1.0 */

We would need to explain this change a bit better.

>  imap-send.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/imap-send.c b/imap-send.c
> index 3847fd1..10dd025 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
>  	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
>  	return -1;
>  #else
> -	SSL_METHOD *meth;
> +	const SSL_METHOD *meth;
>  	SSL_CTX *ctx;
>  	int ret;
>  
> -- 
> 1.6.5.2

Re: [PATCH] imap-send.c: fix pointer to be const

[Vietor Liu]

On Tue, 2009-10-27 at 23:26 -0700, Junio C Hamano wrote:
> Vietor Liu <vietor@vxwo.org> writes:
> 
> > Fixes some compiler warnings:
> > imap-send.c: In function ‘ssl_socket_connect’:
> > warning: assignment discards qualifiers from pointer target type
> >
> > Signed-off-by: Vietor Liu <vietor@vxwo.org>
> 
> I do not quite understand.  This variable gets assigned the return values
> from TLSv1_method() or SSLv23_method(), but the copy of ssl.h I have
> declares them as:
> 
>     SSL_METHOD *SSLv23_method(void);	/* SSLv3 but can rollback to v2 */
>     SSL_METHOD *TLSv1_method(void);		/* TLSv1.0 */

1. openssl-devel-1.0.0-0.10

const SSL_METHOD *SSLv23_method(void);	/* SSLv3 but can rollback to v2
*/
const SSL_METHOD *TLSv1_method(void);		/* TLSv1.0 */


2. http://www.openssl.org/docs/ssl/ssl.html

const SSL_METHOD *SSLv2_method(void);
        
        Constructor for the SSLv2 SSL_METHOD structure for combined
        client and server.
const SSL_METHOD *TLSv1_method(void);
        
        Constructor for the TLSv1 SSL_METHOD structure for combined
        client and server.

3. it maybe fixes warnings for other version.

Re: [PATCH] imap-send.c: fix pointer to be const

[Michael J Gruber]

Vietor Liu venit, vidit, dixit 28.10.2009 08:05:
> On Tue, 2009-10-27 at 23:26 -0700, Junio C Hamano wrote:
>> Vietor Liu <vietor@vxwo.org> writes:
>>
>>> Fixes some compiler warnings:
>>> imap-send.c: In function ‘ssl_socket_connect’:
>>> warning: assignment discards qualifiers from pointer target type
>>>
>>> Signed-off-by: Vietor Liu <vietor@vxwo.org>
>>
>> I do not quite understand.  This variable gets assigned the return values
>> from TLSv1_method() or SSLv23_method(), but the copy of ssl.h I have
>> declares them as:
>>
>>     SSL_METHOD *SSLv23_method(void);	/* SSLv3 but can rollback to v2 */
>>     SSL_METHOD *TLSv1_method(void);		/* TLSv1.0 */
> 
> 1. openssl-devel-1.0.0-0.10
> 
> const SSL_METHOD *SSLv23_method(void);	/* SSLv3 but can rollback to v2
> */
> const SSL_METHOD *TLSv1_method(void);		/* TLSv1.0 */
> 
> 
> 2. http://www.openssl.org/docs/ssl/ssl.html
> 
> const SSL_METHOD *SSLv2_method(void);
>         
>         Constructor for the SSLv2 SSL_METHOD structure for combined
>         client and server.
> const SSL_METHOD *TLSv1_method(void);
>         
>         Constructor for the TLSv1 SSL_METHOD structure for combined
>         client and server.
> 
> 3. it maybe fixes warnings for other version.

No const here with openssl 0.9.8k. I think major distros will switch to
1.0.0 with their next major release (e.g. Fedora 12 will have it by the
end of this year).

Since this is only about warnings, maybe git 1.7.0 is the right time
frame to adjust this to the upcoming standard?

Michael

Re: [PATCH] imap-send.c: fix pointer to be const

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Since this is only about warnings, maybe git 1.7.0 is the right time
> frame to adjust this to the upcoming standard?

This does not look like "one group wants this way, but the others want
differently.  We have to pick one and sacrifice the other because it is
impossible to have it both ways"; there is no excuse to bring up 1.7.0 for
something like this.

Doesn't inclusing "ssl.h" give us some indication whether "const" is
needed to allow us to use #if/#else/#endif in order to compile with
headers from either versions?  I.e. something like...

diff --git a/imap-send.c b/imap-send.c
index 3847fd1..a199db8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -273,7 +273,11 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
 	return -1;
 #else
+#if (OPENSSL_VERSION_NUMBER >= 0x1000000fL)
+	const SSL_METHOD *meth;
+#else
 	SSL_METHOD *meth;
+#endif
 	SSL_CTX *ctx;
 	int ret;
 

Re: [PATCH] imap-send.c: fix pointer to be const

[Vietor Liu]

On Wed, 2009-10-28 at 10:56 -0700, Junio C Hamano wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> > Since this is only about warnings, maybe git 1.7.0 is the right time
> > frame to adjust this to the upcoming standard?
> 
> This does not look like "one group wants this way, but the others want
> differently.  We have to pick one and sacrifice the other because it is
> impossible to have it both ways"; there is no excuse to bring up 1.7.0 for
> something like this.
> 
> Doesn't inclusing "ssl.h" give us some indication whether "const" is
> needed to allow us to use #if/#else/#endif in order to compile with
> headers from either versions?  I.e. something like...
> 
> diff --git a/imap-send.c b/imap-send.c
> index 3847fd1..a199db8 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -273,7 +273,11 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
>  	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
>  	return -1;
>  #else
> +#if (OPENSSL_VERSION_NUMBER >= 0x1000000fL)
> +	const SSL_METHOD *meth;
> +#else
>  	SSL_METHOD *meth;
> +#endif
>  	SSL_CTX *ctx;
>  	int ret;
>  

It's better than my patch, thanks.

Re: [PATCH] imap-send.c: fix pointer to be const

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Vietor Liu <vietor@vxwo.org> writes:
>
>> Fixes some compiler warnings:
>> imap-send.c: In function ‘ssl_socket_connect’:
>> warning: assignment discards qualifiers from pointer target type
>>
>> ====================================================
>> OpenSSL Changes between 0.9.8k and 1.0:
>>
>> *) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
>>      pointer and make the SSL_METHOD parameter in SSL_CTX_new,
>>      SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
>>
>> Signed-off-by: Vietor Liu <vietor@vxwo.org>
>
> This is much easier to understand.

This indeed _is_ easier to understand, but what this means is that making
"meth" const will break the compilation for people with 0.9.8k.

Their SSL_CTX_new() does not promise the callers that it won't modify the
object its parameter points at, so SSL_CTX_new(meth) will now see exactly
the same issue.  You will be discarding "const" qualifier by passing it.

Re: [PATCH] imap-send.c: fix pointer to be const

[Junio C Hamano]

Vietor Liu <vietor@vxwo.org> writes:

> Fixes some compiler warnings:
> imap-send.c: In function ‘ssl_socket_connect’:
> warning: assignment discards qualifiers from pointer target type
>
> ====================================================
> OpenSSL Changes between 0.9.8k and 1.0:
>
> *) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
>      pointer and make the SSL_METHOD parameter in SSL_CTX_new,
>      SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.
>
> Signed-off-by: Vietor Liu <vietor@vxwo.org>

This is much easier to understand.

Thanks.

> ---
>  imap-send.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/imap-send.c b/imap-send.c
> index 3847fd1..10dd025 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
>  	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
>  	return -1;
>  #else
> -	SSL_METHOD *meth;
> +	const SSL_METHOD *meth;
>  	SSL_CTX *ctx;
>  	int ret;
>  
> -- 
> 1.6.5.2

[PATCH] imap-send.c: fix pointer to be const

[Vietor Liu]

Fixes some compiler warnings:
imap-send.c: In function ‘ssl_socket_connect’:
warning: assignment discards qualifiers from pointer target type

====================================================
OpenSSL Changes between 0.9.8k and 1.0:

*) Let the TLSv1_method() etc. functions return a 'const' SSL_METHOD
     pointer and make the SSL_METHOD parameter in SSL_CTX_new,
     SSL_CTX_set_ssl_version and SSL_set_ssl_method 'const'.

Signed-off-by: Vietor Liu <vietor@vxwo.org>
---
 imap-send.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 3847fd1..10dd025 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -273,7 +273,7 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
 	return -1;
 #else
-	SSL_METHOD *meth;
+	const SSL_METHOD *meth;
 	SSL_CTX *ctx;
 	int ret;
 
-- 
1.6.5.2