[Spice-devel] [PATCH spice-common v3] build: Remove FIXME_SERVER_SMARTCARD hack

[Eduardo Lima (Etrunko)]

When we remove the hacks in configure.ac and common/Makefile.am, two
errors pop out:

generated_server_demarshallers.c: In function ‘parse_msgc_smartcard_reader_add’:
generated_server_demarshallers.c:1985:30: error: ‘mem_size’ undeclared (first use in this function); did you mean ‘nw_size’?
     data = (uint8_t *)malloc(mem_size);
                              ^~~~~~~~
                              nw_size

First one is caused by a missing declaration of mem_size, so we use the
same condition that causes this code to be added to the check for the
need of mem_size variable declaration in demarshal.py.

generated_server_demarshallers.c:1985:30: note: each undeclared identifier is reported only once for each function it appears in
generated_server_demarshallers.c:1994:15: error: ‘VSCMsgReaderAdd {aka struct VSCMsgReaderAdd}’ has no member named ‘reader_name’
     memcpy(out->reader_name, in, reader_name__nelements);
               ^~

This second one is only a rename of 'reader_name' field to 'name', as
specified in the VSCMsgReaderAdd structure in file vscard_common.h.

N.B.: Even with those errors fixed, it looks like the generated code is
really broken and should not be built at all. So, following suggestion
after review in the mailing list, this patch also changes the define in
the code to be different from the one that is used to tell whether the
smartcard dependencies are found.

https://lists.freedesktop.org/archives/spice-devel/2018-May/043395.html

On 10/05/18 12:01, Frediano Ziglio wrote:
> I would just use the same ugly workaround.
> Do not define USE_SMARTCARD!
> (or change @ifdef(USE_SMARTCARD) to @ifdef(I_REALLY_WANT_TO_USE_THIS_BROKEN_CODE))
>
> The spice.proto for smart card is broken!
> First there are 4 message with same code (ok, this should be checked
> but is clearly an error).
> Second the messages are not defined in the same way they are encoded
> (manually!) by the current code (client and server).
> Apparently somebody try to use the protocol file and ended up
> commenting out the part relative to smart card.
> I got a working version but I don't like either, seems the combination
> required is not really handled by the Python code.

Signed-off-by: Eduardo Lima (Etrunko) <etrunko at redhat.com>
---
 common/Makefile.am          |  2 --
 configure.ac                |  7 -------
 python_modules/demarshal.py |  3 +--
 spice.proto                 | 15 +++++++++++++--
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/common/Makefile.am b/common/Makefile.am
index 22aec80..ea15039 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -78,8 +78,6 @@ libspice_common_server_la_SOURCES =		\
 	$(SERVER_MARSHALLERS)			\
 	$(NULL)
 
-libspice_common_server_la_CFLAGS = -DFIXME_SERVER_SMARTCARD
-
 AM_CPPFLAGS =				\
 	-I$(top_srcdir)			\
 	-I$(top_builddir)		\
diff --git a/configure.ac b/configure.ac
index 3da85de..39faf1c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -64,11 +64,4 @@ AC_CONFIG_FILES([
   docs/Makefile
 ])
 
-AH_BOTTOM([
-/* argh.. this is evil */
-#if defined(FIXME_SERVER_SMARTCARD) && defined(USE_SMARTCARD)
-%:undef USE_SMARTCARD
-#endif
-])
-
 AC_OUTPUT
diff --git a/python_modules/demarshal.py b/python_modules/demarshal.py
index da87d44..dc7a815 100644
--- a/python_modules/demarshal.py
+++ b/python_modules/demarshal.py
@@ -1041,8 +1041,7 @@ def write_msg_parser(writer, message):
     msg_type = message.c_type()
     msg_sizeof = message.sizeof()
 
-    want_mem_size = (len(message.members) != 1 or message.members[0].is_fixed_nw_size()
-                         or not message.members[0].is_array())
+    want_mem_size = not message.has_attr("nocopy")
 
     writer.newline()
     if message.has_attr("ifdef"):
diff --git a/spice.proto b/spice.proto
index 6f873a2..baa2eec 100644
--- a/spice.proto
+++ b/spice.proto
@@ -1423,9 +1423,20 @@ channel SmartcardChannel : BaseChannel {
     } @ctype(VSCMsgATR) atr = 101;
 
     message {
-	int8 reader_name[] @zero_terminated @nonnull;
+	int8 name[] @zero_terminated @nonnull;
     } @ctype(VSCMsgReaderAdd) reader_add = 101;
-} @ifdef(USE_SMARTCARD);
+
+/* The protocol specification for SmartcardChannel as it is, does not work:
+ * - All messages have the same ID, which causes some message definitions
+ *   to be ignored;
+ * - The structure of the resulting message is not the same used by the code,
+ *   so it would require a change in the protocol.
+ *
+ * For the above exposed, we still protect this code with #ifdef guards, but
+ * using a different define than the one used to check if the smartcard
+ * dependencies are found.
+ */
+} @ifdef(FIXME_SERVER_SMARTCARD); //@ifdef(USE_SMARTCARD);
 
 channel SpicevmcChannel : BaseChannel {
 server:
-- 
2.14.3