Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-09-05 Thread Daniel P. Berrange 
On Mon, Sep 05, 2016 at 12:02:42PM +, Gonglei (Arei) wrote:
> 
> > -Original Message-
> > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > Sent: Monday, September 05, 2016 7:04 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org
> > Subject: Re: [PATCH v2 1/2] crypto: fix building complaint
> > 
> > On Mon, Sep 05, 2016 at 10:50:54AM +, Gonglei (Arei) wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > > > Sent: Monday, September 05, 2016 6:15 PM
> > > > To: Gonglei (Arei)
> > > > Cc: qemu-devel@nongnu.org; dmi...@daynix.com
> > > > Subject: Re: [PATCH v2 1/2] crypto: fix building complaint
> > > >
> > > > On Tue, Aug 30, 2016 at 12:10:52PM +0800, Gonglei wrote:
> > > > > gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to
> > > > GNUTLS_VERSION_NUMBER.
> > > > > If using gnutls before that verion, we'll get the below warning:
> > > > > crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is
> > not
> > > > defined
> > > > >
> > > > > Signed-off-by: Gonglei 
> > > > > ---
> > > > >  crypto/tlscredsx509.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> > > > > index 520d34d..f2fd80f 100644
> > > > > --- a/crypto/tlscredsx509.c
> > > > > +++ b/crypto/tlscredsx509.c
> > > > > @@ -615,7 +615,7 @@
> > qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> > > > *creds,
> > > > >  }
> > > > >
> > > > >  if (cert != NULL && key != NULL) {
> > > > > -#if GNUTLS_VERSION_NUMBER >= 0x030111
> > > > > +#if defined(GNUTLS_VERSION_NUMBER) &&
> > > > GNUTLS_VERSION_NUMBER >= 0x030111
> > > > >  char *password = NULL;
> > > > >  if (creds->passwordid) {
> > > > >  password =
> > > > qcrypto_secret_lookup_as_utf8(creds->passwordid,
> > > >
> > > > Other places in this file still use LIBGNUTLS_VERSION_NUMBER, which is
> > > > good, because gnutls 3.x still defines LIBGNUTLS_VERSION_NUMBER for
> > > > back compat.
> > > >
> > > You mean using LIBGNUTLS_VERSION_NUMBER directly here? That's ok.
> > 
> > Yes,
> > 
> > > > The tests/test-crypto-tlscredsx509.c file also needs a similar fix.
> > > >
> > > Sorry?
> > 
> > It also uses the GNUTLS_VERSION_NUMBER constant instead of
> > LIBGNUTLS_VERSION_NUMBER
> > 
> 
> What about the below patch?
> 
> diff --git a/crypto/init.c b/crypto/init.c
> index 1e564d9..16e099b 100644
> --- a/crypto/init.c
> +++ b/crypto/init.c
> @@ -59,8 +59,7 @@
>  
>  #if (defined(CONFIG_GCRYPT) &&  \
>   (!defined(CONFIG_GNUTLS) ||\
> -  !defined(GNUTLS_VERSION_NUMBER) ||   \
> -  (GNUTLS_VERSION_NUMBER < 0x020c00)) &&\
> + (LIBGNUTLS_VERSION_NUMBER < 0x020c00)) &&\
>   (!defined(GCRYPT_VERSION_NUMBER) ||\
>(GCRYPT_VERSION_NUMBER < 0x010600)))
>  #define QCRYPTO_INIT_GCRYPT_THREADS
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 520d34d..50eb54f 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -615,7 +615,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
>  }
>  
>  if (cert != NULL && key != NULL) {
> -#if GNUTLS_VERSION_NUMBER >= 0x030111
> +#if LIBGNUTLS_VERSION_NUMBER >= 0x030111
>  char *password = NULL;
>  if (creds->passwordid) {
>  password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
> @@ -630,7 +630,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
>  password,
>  0);
>  g_free(password);
> -#else /* GNUTLS_VERSION_NUMBER < 0x030111 */
> +#else /* LIBGNUTLS_VERSION_NUMBER < 0x030111 */
>  if (creds->passwordid) {
>  error_setg(errp, "PKCS8 decryption requires GNUTLS >= 3.1.11");
>  goto cleanup;
> @@ -638,7 +638,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
>  ret = gnutls_certificate_set_x509_key_file(creds->data,
> cert, key,
> GNUTLS_X509_FMT_PEM);
> -#endif /* GNUTLS_VERSION_NUMBER < 0x030111 */
> +#endif
>  if (ret < 0) {
>  error_setg(errp, "Cannot load certificate '%s' & key '%s': %s",
> cert, key, gnutls_strerror(ret));
> diff --git a/tests/crypto-tls-x509-helpers.h b/tests/crypto-tls-x509-helpers.h
> index 356b49c..a8faa92 100644
> --- a/tests/crypto-tls-x509-helpers.h
> +++ b/tests/crypto-tls-x509-helpers.h
> @@ -26,7 +26,6 @@
>  
>  #if !(defined WIN32) && \
>  defined(CONFIG_TASN1) && \
> -defined(LIBGNUTLS_VERSION_NUMBER) && \
>  (LIBGNUTLS_VERSION_NUMBER >= 0x020600)
>  # define QCRYPTO_HAVE_TLS_TEST_SUPPORT
>  #endif

Yep, that looks reasonable.

Regards,
Daniel
-- 
|: http://berrange.com  -o-

Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-09-05 Thread Gonglei (Arei) 

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Monday, September 05, 2016 7:04 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org
> Subject: Re: [PATCH v2 1/2] crypto: fix building complaint
> 
> On Mon, Sep 05, 2016 at 10:50:54AM +, Gonglei (Arei) wrote:
> >
> >
> > > -Original Message-
> > > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > > Sent: Monday, September 05, 2016 6:15 PM
> > > To: Gonglei (Arei)
> > > Cc: qemu-devel@nongnu.org; dmi...@daynix.com
> > > Subject: Re: [PATCH v2 1/2] crypto: fix building complaint
> > >
> > > On Tue, Aug 30, 2016 at 12:10:52PM +0800, Gonglei wrote:
> > > > gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to
> > > GNUTLS_VERSION_NUMBER.
> > > > If using gnutls before that verion, we'll get the below warning:
> > > > crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is
> not
> > > defined
> > > >
> > > > Signed-off-by: Gonglei 
> > > > ---
> > > >  crypto/tlscredsx509.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> > > > index 520d34d..f2fd80f 100644
> > > > --- a/crypto/tlscredsx509.c
> > > > +++ b/crypto/tlscredsx509.c
> > > > @@ -615,7 +615,7 @@
> qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> > > *creds,
> > > >  }
> > > >
> > > >  if (cert != NULL && key != NULL) {
> > > > -#if GNUTLS_VERSION_NUMBER >= 0x030111
> > > > +#if defined(GNUTLS_VERSION_NUMBER) &&
> > > GNUTLS_VERSION_NUMBER >= 0x030111
> > > >  char *password = NULL;
> > > >  if (creds->passwordid) {
> > > >  password =
> > > qcrypto_secret_lookup_as_utf8(creds->passwordid,
> > >
> > > Other places in this file still use LIBGNUTLS_VERSION_NUMBER, which is
> > > good, because gnutls 3.x still defines LIBGNUTLS_VERSION_NUMBER for
> > > back compat.
> > >
> > You mean using LIBGNUTLS_VERSION_NUMBER directly here? That's ok.
> 
> Yes,
> 
> > > The tests/test-crypto-tlscredsx509.c file also needs a similar fix.
> > >
> > Sorry?
> 
> It also uses the GNUTLS_VERSION_NUMBER constant instead of
> LIBGNUTLS_VERSION_NUMBER
> 

What about the below patch?

diff --git a/crypto/init.c b/crypto/init.c
index 1e564d9..16e099b 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -59,8 +59,7 @@
 
 #if (defined(CONFIG_GCRYPT) &&  \
  (!defined(CONFIG_GNUTLS) ||\
-  !defined(GNUTLS_VERSION_NUMBER) ||   \
-  (GNUTLS_VERSION_NUMBER < 0x020c00)) &&\
+ (LIBGNUTLS_VERSION_NUMBER < 0x020c00)) &&\
  (!defined(GCRYPT_VERSION_NUMBER) ||\
   (GCRYPT_VERSION_NUMBER < 0x010600)))
 #define QCRYPTO_INIT_GCRYPT_THREADS
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 520d34d..50eb54f 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -615,7 +615,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
 }
 
 if (cert != NULL && key != NULL) {
-#if GNUTLS_VERSION_NUMBER >= 0x030111
+#if LIBGNUTLS_VERSION_NUMBER >= 0x030111
 char *password = NULL;
 if (creds->passwordid) {
 password = qcrypto_secret_lookup_as_utf8(creds->passwordid,
@@ -630,7 +630,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
 password,
 0);
 g_free(password);
-#else /* GNUTLS_VERSION_NUMBER < 0x030111 */
+#else /* LIBGNUTLS_VERSION_NUMBER < 0x030111 */
 if (creds->passwordid) {
 error_setg(errp, "PKCS8 decryption requires GNUTLS >= 3.1.11");
 goto cleanup;
@@ -638,7 +638,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
 ret = gnutls_certificate_set_x509_key_file(creds->data,
cert, key,
GNUTLS_X509_FMT_PEM);
-#endif /* GNUTLS_VERSION_NUMBER < 0x030111 */
+#endif
 if (ret < 0) {
 error_setg(errp, "Cannot load certificate '%s' & key '%s': %s",
cert, key, gnutls_strerror(ret));
diff --git a/tests/crypto-tls-x509-helpers.h b/tests/crypto-tls-x509-helpers.h
index 356b49c..a8faa92 100644
--- a/tests/crypto-tls-x509-helpers.h
+++ b/tests/crypto-tls-x509-helpers.h
@@ -26,7 +26,6 @@
 
 #if !(defined WIN32) && \
 defined(CONFIG_TASN1) && \
-defined(LIBGNUTLS_VERSION_NUMBER) && \
 (LIBGNUTLS_VERSION_NUMBER >= 0x020600)
 # define QCRYPTO_HAVE_TLS_TEST_SUPPORT
 #endif


> Regards,
> Daniel
> --
> |: http://berrange.com  -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o-
> http://virt-manager.org :|
> |: http://autobuild.org   -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-
> http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-09-05 Thread Daniel P. Berrange 
On Mon, Sep 05, 2016 at 10:50:54AM +, Gonglei (Arei) wrote:
> 
> 
> > -Original Message-
> > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > Sent: Monday, September 05, 2016 6:15 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; dmi...@daynix.com
> > Subject: Re: [PATCH v2 1/2] crypto: fix building complaint
> > 
> > On Tue, Aug 30, 2016 at 12:10:52PM +0800, Gonglei wrote:
> > > gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to
> > GNUTLS_VERSION_NUMBER.
> > > If using gnutls before that verion, we'll get the below warning:
> > > crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is not
> > defined
> > >
> > > Signed-off-by: Gonglei 
> > > ---
> > >  crypto/tlscredsx509.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> > > index 520d34d..f2fd80f 100644
> > > --- a/crypto/tlscredsx509.c
> > > +++ b/crypto/tlscredsx509.c
> > > @@ -615,7 +615,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> > *creds,
> > >  }
> > >
> > >  if (cert != NULL && key != NULL) {
> > > -#if GNUTLS_VERSION_NUMBER >= 0x030111
> > > +#if defined(GNUTLS_VERSION_NUMBER) &&
> > GNUTLS_VERSION_NUMBER >= 0x030111
> > >  char *password = NULL;
> > >  if (creds->passwordid) {
> > >  password =
> > qcrypto_secret_lookup_as_utf8(creds->passwordid,
> > 
> > Other places in this file still use LIBGNUTLS_VERSION_NUMBER, which is
> > good, because gnutls 3.x still defines LIBGNUTLS_VERSION_NUMBER for
> > back compat.
> > 
> You mean using LIBGNUTLS_VERSION_NUMBER directly here? That's ok.

Yes,

> > The tests/test-crypto-tlscredsx509.c file also needs a similar fix.
> > 
> Sorry?

It also uses the GNUTLS_VERSION_NUMBER constant instead of
LIBGNUTLS_VERSION_NUMBER

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-09-05 Thread Gonglei (Arei) 


> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Monday, September 05, 2016 6:15 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; dmi...@daynix.com
> Subject: Re: [PATCH v2 1/2] crypto: fix building complaint
> 
> On Tue, Aug 30, 2016 at 12:10:52PM +0800, Gonglei wrote:
> > gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to
> GNUTLS_VERSION_NUMBER.
> > If using gnutls before that verion, we'll get the below warning:
> > crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is not
> defined
> >
> > Signed-off-by: Gonglei 
> > ---
> >  crypto/tlscredsx509.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> > index 520d34d..f2fd80f 100644
> > --- a/crypto/tlscredsx509.c
> > +++ b/crypto/tlscredsx509.c
> > @@ -615,7 +615,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509
> *creds,
> >  }
> >
> >  if (cert != NULL && key != NULL) {
> > -#if GNUTLS_VERSION_NUMBER >= 0x030111
> > +#if defined(GNUTLS_VERSION_NUMBER) &&
> GNUTLS_VERSION_NUMBER >= 0x030111
> >  char *password = NULL;
> >  if (creds->passwordid) {
> >  password =
> qcrypto_secret_lookup_as_utf8(creds->passwordid,
> 
> Other places in this file still use LIBGNUTLS_VERSION_NUMBER, which is
> good, because gnutls 3.x still defines LIBGNUTLS_VERSION_NUMBER for
> back compat.
> 
You mean using LIBGNUTLS_VERSION_NUMBER directly here? That's ok.

> The tests/test-crypto-tlscredsx509.c file also needs a similar fix.
> 
Sorry?


Regards,
-Gonglei


> Regards,
> Daniel
> --
> |: http://berrange.com  -o-
> http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o-
> http://virt-manager.org :|
> |: http://autobuild.org   -o-
> http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-
> http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-09-05 Thread Daniel P. Berrange 
On Tue, Aug 30, 2016 at 12:10:52PM +0800, Gonglei wrote:
> gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to 
> GNUTLS_VERSION_NUMBER.
> If using gnutls before that verion, we'll get the below warning:
> crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is not defined
> 
> Signed-off-by: Gonglei 
> ---
>  crypto/tlscredsx509.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 520d34d..f2fd80f 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -615,7 +615,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
>  }
>  
>  if (cert != NULL && key != NULL) {
> -#if GNUTLS_VERSION_NUMBER >= 0x030111
> +#if defined(GNUTLS_VERSION_NUMBER) && GNUTLS_VERSION_NUMBER >= 0x030111
>  char *password = NULL;
>  if (creds->passwordid) {
>  password = qcrypto_secret_lookup_as_utf8(creds->passwordid,

Other places in this file still use LIBGNUTLS_VERSION_NUMBER, which is
good, because gnutls 3.x still defines LIBGNUTLS_VERSION_NUMBER for
back compat.

The tests/test-crypto-tlscredsx509.c file also needs a similar fix.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-08-30 Thread Peter Maydell 
On 30 August 2016 at 15:14, Eric Blake  wrote:
> On 08/29/2016 11:10 PM, Gonglei wrote:
>> gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to 
>> GNUTLS_VERSION_NUMBER.
>> If using gnutls before that verion, we'll get the below warning:
>
> s/verion/version/
>
>> crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is not defined
>>
>
> The C preprocessor is well-defined to treat an unknown macro name as 0.
> Which compiler are you using that complains?  Is this something that we
> consistently work around, or is it better to figure out how to fix your
> compiler to quit complaining?

This is gcc's -Wundef, which we explicitly enable in configure.
(The rationale is the usual one, ie that almost all the time this
happens it's by accident and so it's worth putting in the extra
effort to avoid the odd case where you were doing it deliberately.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] crypto: fix building complaint

2016-08-30 Thread Eric Blake 
On 08/29/2016 11:10 PM, Gonglei wrote:
> gnutls commit 846753877d renamed LIBGNUTLS_VERSION_NUMBER to 
> GNUTLS_VERSION_NUMBER.
> If using gnutls before that verion, we'll get the below warning:

s/verion/version/

> crypto/tlscredsx509.c:618:5: warning: "GNUTLS_VERSION_NUMBER" is not defined
> 

The C preprocessor is well-defined to treat an unknown macro name as 0.
Which compiler are you using that complains?  Is this something that we
consistently work around, or is it better to figure out how to fix your
compiler to quit complaining?

> Signed-off-by: Gonglei 
> ---
>  crypto/tlscredsx509.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 520d34d..f2fd80f 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -615,7 +615,7 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
>  }
>  
>  if (cert != NULL && key != NULL) {
> -#if GNUTLS_VERSION_NUMBER >= 0x030111
> +#if defined(GNUTLS_VERSION_NUMBER) && GNUTLS_VERSION_NUMBER >= 0x030111

At any rate, this is the correct way to silence the warning (even if
verbose), if we do want it, so:
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature