Re: [Qemu-devel] [PATCH 0/6] usb-ccid (v8)

[Blue Swirl]

On Sun, Dec 12, 2010 at 11:35 AM, Alon Levy <address@hidden> wrote:
> On Sat, Dec 11, 2010 at 03:55:10PM +0000, Blue Swirl wrote:
>> On Sat, Dec 11, 2010 at 3:33 PM, Hans de Goede <address@hidden> wrote:
>> > Hi,
>> >
>> > On 12/11/2010 10:43 AM, Blue Swirl wrote:
>> >>
>> >> On Tue, Dec 7, 2010 at 10:20 AM, Alon Levy<address@hidden>  wrote:
>> >>>
>> >>> ping.
>> >>
>> >> I don't think Anthony's concerns (or mine) have been addressed.
>> >>
>> >
>> > Could you be a bit more verbose please ?
>>
>> I'm not sure if this answer was OK for Anthony given the previous discussion:
>> http://article.gmane.org/gmane.comp.emulators.qemu/85793
>>
>
> I'm pretty sure it's ok by anthony since he suggested the inclusion of
> libcacard in qemu in the first place. (that's not a typo - I've renamed
> it to avoid the double card, I'll send an updated patch merging 4/6 and
> including this rename).
>
>> About my concerns, see my previous mail about merging the shared
>> library fix to 4/6 and about other stuff that does not belong to QEMU
>> but which should be a separate project.
>>
> your concerns were:
>  usb-ccid.c:
>  endianess - I've added network to host convertions (used as guest to host).
>  coding style - answer_t and bulk_in_t renamed
>  hungarian style usage - gave 'excuse' (spec uses those names)
>  anonymous structs usage - named
>  libcaccard:
>  don't build a library that qemu doesn't use: v8.1, will fold in and send v9
>
> Am I missing anything?

No. But now that I tried to build it, there are other problems.

There is a typo in configure:
diff --git a/configure b/configure
index 4b55904..7288b09 100755
--- a/configure
+++ b/configure
@@ -2142,7 +2142,7 @@ if test "$smartcard" != "no" ; then
     QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcaccard_cflags"
     LIBS="$libcaccard_libs $LIBS"
   else
-    if test "smartcard" = "yes" ; then
+    if test "$smartcard" = "yes" ; then
       feature_not_found "smartcard"
     fi
     smartcard="no"

Build also fails when compiling in an object directory:
make: *** libcaccard: No such file or directory.  Stop.
make: *** [subdir-libcaccard] Error 2

I'd even remove the configure option and build the card by default if
the dependencies are found.

There were also whitespace problems when applying:
Applying ccid: add ccid-card-emulated device (v2)
.dotest/patch:37: trailing whitespace.
 *  for i in 1 2 3; do certutil -d /etc/pki/nssdb -x -t "CT,CT,CT" -S
-s "CN=user$i" -n user$i; done
warning: 1 line applied after fixing whitespace errors.
Applying ccid: add docs
.dotest/patch:121: trailing whitespace.
                            [APDU<->APDU repeats several times]
warning: 1 line applied after fixing whitespace errors.