Patchwork [0/6] usb-ccid (v8)

login
register
mail settings
Submitter Blue Swirl
Date Dec. 12, 2010, 3:07 p.m.
Message ID <AANLkTi=27DG-Tx_dy3cBVX2XVzkAio+F8ksq4zrCmBgi@mail.gmail.com>
Download mbox | patch
Permalink /patch/75253/
State New
Headers show

Comments

Blue Swirl - Dec. 12, 2010, 3:07 p.m.
On Sun, Dec 12, 2010 at 11:35 AM, Alon Levy <alevy@redhat.com> 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 <hdegoede@redhat.com> wrote:
>> > Hi,
>> >
>> > On 12/11/2010 10:43 AM, Blue Swirl wrote:
>> >>
>> >> On Tue, Dec 7, 2010 at 10:20 AM, Alon Levy<alevy@redhat.com>  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:

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.
Alon Levy - Dec. 12, 2010, 4:24 p.m.
On Sun, Dec 12, 2010 at 03:07:52PM +0000, Blue Swirl wrote:
<snip>
> 
> 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
> 
ok, will fix.

> I'd even remove the configure option and build the card by default if
> the dependencies are found.
> 
ok, good idea.

> 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.
> 
will fix as well.

Patch

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"