Re: [Xen-devel] [PATH 9/12] VTPM mini-os: New stubdom applications

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Matthew Fioravante, le Fri 11 Mar 2011 18:06:26 -0500, a écrit :
> +-#ifdef HAVE_LONG_DOUBLE
> +-#define LDOUBLE long double
> +-#else
> +-#define LDOUBLE double
> +-#endif
>....

Mmm, I'd rather use #if 0 / #endif instead in such case, so that merging
newer upstream versions is way easier.

> +-#if defined(OPENSSL_SYS_WIN32) || defined(OPENSSL_SYS_VMS) || 
> defined(OPENSSL_SYS_MSDOS) || defined(OPENSSL_SYS_VXWORKS) || 
> defined(OPENSSL_SYS_NETWARE) || defined(OPENSSL_SYS_VOS) || 
> defined(OPENSSL_SYS_BEOS)
> + int RAND_query_egd_bytes(const char *path, unsigned char *buf, int bytes)
> +     {
> +     return(-1);
> +@@ -109,195 +108,3 @@
> +     {
> +     return(-1);
> +     }
> +-#else

Same remark: simply replace the test with #if 1, so you don't need to
drop the #else part. Actually, you could even talk with upstream to get
#ifdef __MINIOS__ even commited there.

> +diff -Naur polarssl-0.12.1-pristine/library/timing.c 
> polarssl-0.12.1/library/timing.c
> +--- polarssl-0.12.1-pristine/library/timing.c        2010-11-03 
> 14:50:09.000000000 -0400
> ++++ polarssl-0.12.1/library/timing.c 2010-11-04 10:30:56.000000000 -0400
> +@@ -42,6 +42,7 @@
> + #include <unistd.h>
> + #include <sys/types.h>
> + #include <sys/time.h>
> ++#include <sys/select.h>
> + #include <signal.h>
> + #include <time.h>

Maybe we should rather check which potential posix compatibility is
missing here?

> diff -Naur xen-unstable-trp-sdp-pristine/stubdom/tpmemu-0.6.1.patch 
> xen-unstable-trp-sdp/stubdom/tpmemu-0.6.1.patch
> --- xen-unstable-trp-sdp-pristine/stubdom/tpmemu-0.6.1.patch  1969-12-31 
> 19:00:00.000000000 -0500
> +++ xen-unstable-trp-sdp/stubdom/tpmemu-0.6.1.patch   2011-02-23 
> 19:40:48.000000000 -0500
> @@ -0,0 +1,78 @@
> +diff -Naur tpm_emulator-0.6.1-pristine/tpm/tpm_data.c 
> tpm_emulator-0.6.1/tpm/tpm_data.c
> +--- tpm_emulator-0.6.1-pristine/tpm/tpm_data.c       2009-10-09 
> 05:07:22.000000000 -0400
> ++++ tpm_emulator-0.6.1/tpm/tpm_data.c        2011-02-18 14:23:16.000000000 
> -0500
> +@@ -246,8 +246,7 @@
> +       || tpm_unmarshal_TPM_PERMANENT_DATA(&ptr, &len, 
> &tpmData.permanent.data)
> +       || tpm_unmarshal_TPM_STCLEAR_FLAGS(&ptr, &len, 
> &tpmData.stclear.flags)      
> +       || tpm_unmarshal_TPM_STCLEAR_DATA(&ptr, &len, &tpmData.stclear.data)
> +-      || tpm_unmarshal_TPM_STANY_DATA(&ptr, &len, &tpmData.stany.data)
> +-      || len > 0) {
> ++      || tpm_unmarshal_TPM_STANY_DATA(&ptr, &len, &tpmData.stany.data)) {
> +     tpm_free(buf);
> +     return -1;
> +   }

Could this actually be a bug somewhere?

> +@@ -85,19 +81,19 @@
> + 
> + #if defined(__linux__)
> + 
> +-#define CPU_TO_BE64(x) __cpu_to_be64(x)
> +-#define CPU_TO_LE64(x) __cpu_to_le64(x)
> +-#define CPU_TO_BE32(x) __cpu_to_be32(x)
> +-#define CPU_TO_LE32(x) __cpu_to_le32(x)
> +-#define CPU_TO_BE16(x) __cpu_to_be16(x)
> +-#define CPU_TO_LE16(x) __cpu_to_le16(x)
> +-
> +-#define BE64_TO_CPU(x) __be64_to_cpu(x)
> +-#define LE64_TO_CPU(x) __le64_to_cpu(x)
> +-#define BE32_TO_CPU(x) __be32_to_cpu(x)
> +-#define LE32_TO_CPU(x) __le32_to_cpu(x)
> +-#define BE16_TO_CPU(x) __be16_to_cpu(x)
> +-#define LE16_TO_CPU(x) __le16_to_cpu(x)
> ++#define CPU_TO_BE64(x) cpu_to_be64(x)
> ++#define CPU_TO_LE64(x) cpu_to_le64(x)
> ++#define CPU_TO_BE32(x) cpu_to_be32(x)

Same here: instead of removing lines which may change upstream, rather
add an #if 0 to comment them out and only add what you need.

> +        make CFLAGS="$(TARGET_CPPFLAGS) $(TARGET_CFLAGS) -I. -I.. -isystem 
> $(CURDIR)/vtpm/compat -D__linux__" CC=$(CC) -C tpmd objs )

I'm not very happy with -D__linux__, since you're actually lying. If
upstream accepts #ifdef __MINIOS__ in their code, it would make things a
lot simpler with further upstream releases.

Samuel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel