On 11/05/18 10:14, Eduardo Lima (Etrunko) wrote:
> 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@xxxxxxxxxx>
> ---
> 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);
I have just noticed that this causes a build failure in spice-gtk, so it
is better to keep the define as is at the moment. At least until we find
somewhat final solution to this issue.
Regards, Eduardo.
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel