[Spice-devel] [PATCH 11/13] smartcard: server side (not enabled yet)

[Alon Levy]

On Tue, Dec 07, 2010 at 12:08:08PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 12/07/2010 11:51 AM, Alon Levy wrote:
> >On Tue, Dec 07, 2010 at 11:34:43AM +0100, Hans de Goede wrote:
> >>Hi,
> >>
> >>One very small nitpick, still: Ack!
> >>
> >>On 12/06/2010 05:16 PM, Alon Levy wrote:
> >>>---
> >>>  server/reds.c      |   26 +++
> >>>  server/smartcard.c |  532 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  server/smartcard.h |   18 ++
> >>>  3 files changed, 576 insertions(+), 0 deletions(-)
> >>>  create mode 100644 server/smartcard.c
> >>>  create mode 100644 server/smartcard.h
> >>>
> >>>diff --git a/server/reds.c b/server/reds.c
> >>>index 517ac6d..9e12073 100644
> >>>--- a/server/reds.c
> >>>+++ b/server/reds.c
> >>
> >><snip>
> >>
> >>>@@ -3432,9 +3436,15 @@ __visible__ void spice_server_char_device_wakeup(SpiceCharDeviceInstance* sin)
> >>>  }
> >>>
> >>>  #define SUBTYPE_VDAGENT "vdagent"
> >>>+#ifdef USE_SMARTCARD
> >>>+#define SUBTYPE_SMARTCARD "smartcard"
> >>>+#endif
> >>>
> >>
> >>I see little value in the #ifdef #endif pair above.
> >
> >The value is that if you compile without smartcard support you will get an
> >error from qemu command line if you try to create a spice chardevice with
> >smartcard as subtype. Better then an error later.
> >
> 
> I know, but that is caused by the #ifdef #endif pair below this line /
> my original comment, I was talking about seeing little value in the
> #ifdef #endif pair above this line / my original comment.
> 
> >>
> >>>  const char *spice_server_char_device_recognized_subtypes_list[] = {
> >>>      SUBTYPE_VDAGENT,
> >>>+#ifdef USE_SMARTCARD
> >>>+    SUBTYPE_SMARTCARD,
> >>>+#endif
> >>>      NULL,
> >>>  };
> >>>
> >>
> 
> Summarizing I'm suggesting to replace:
> 
> +#ifdef USE_SMARTCARD
> +#define SUBTYPE_SMARTCARD "smartcard"
> +#endif
> 
> With:
> 
> +#define SUBTYPE_SMARTCARD "smartcard"
> 
> And keep:
> 
>   const char *spice_server_char_device_recognized_subtypes_list[] = {
>       SUBTYPE_VDAGENT,
> +#ifdef USE_SMARTCARD
> +    SUBTYPE_SMARTCARD,
> +#endif
>       NULL,
>   };

Right, my bad.

> 
> As is.
> 
> Regards,
> 
> Hans
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel