All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch'
@ 2015-03-19 14:53 Thomas Huth
  2015-09-14 12:11 ` Markus Armbruster
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Huth @ 2015-03-19 14:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: alevy, armbru, Thomas Huth

'smatch' complains about two bugs and one style issue in card_7816.c:

libcacard/card_7816.c:273 vcard_apdu_set_length() warn: should this be a bitwise op?
libcacard/card_7816.c:295 vcard_apdu_set_length() warn: should this be a bitwise op?
libcacard/card_7816.c:661 vcard7816_vm_process_apdu() warn: inconsistent indenting

... and indeed, the code seems to be wrong here. Let's fix this
by using a bitwise OR instead of logical OR and by indenting
the code with the right level.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
---
Please note that this is compile-tested only. I don't have a clue
about that libcacard stuff, so if you feel confident in this area,
please have a look whether this change really makes sense.
---
 libcacard/card_7816.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
index 814fa16..0f7a006 100644
--- a/libcacard/card_7816.c
+++ b/libcacard/card_7816.c
@@ -270,7 +270,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
             }
             /* calculate the first extended value. Could be either Le or Lc */
             Le = (apdu->a_header->ah_body[0] << 8)
-               || apdu->a_header->ah_body[1];
+                | apdu->a_header->ah_body[1];
             if (L == 3) {
                 /* 2E extended, return data only */
                 /*   zero maps to 65536 */
@@ -292,7 +292,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
             if (L == Le+5) {
                 /* 4E extended, parameters and return data */
                 Le = (apdu->a_data[apdu->a_len-2] << 8)
-                   || apdu->a_data[apdu->a_len-1];
+                    | apdu->a_data[apdu->a_len-1];
                 apdu->a_Le = Le ? Le : 65536;
                 return VCARD7816_STATUS_SUCCESS;
             }
@@ -657,7 +657,7 @@ vcard7816_vm_process_apdu(VCard *card, VCardAPDU *apdu,
                     }
                 }
             } else {
-                    status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
+                status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
                 *response = vcard_make_response(status);
             }
         }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch'
  2015-03-19 14:53 [Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch' Thomas Huth
@ 2015-09-14 12:11 ` Markus Armbruster
  2015-09-14 12:46   ` Marc-André Lureau
  0 siblings, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2015-09-14 12:11 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Jeremy White, Marc-André Lureau, qemu-devel

Stumbled over this while throwing out old mail.  Copying Marc-André and
Jeremy.

Thomas Huth <thuth@linux.vnet.ibm.com> writes:

> 'smatch' complains about two bugs and one style issue in card_7816.c:
>
> libcacard/card_7816.c:273 vcard_apdu_set_length() warn: should this be a bitwise op?
> libcacard/card_7816.c:295 vcard_apdu_set_length() warn: should this be a bitwise op?
> libcacard/card_7816.c:661 vcard7816_vm_process_apdu() warn: inconsistent indenting
>
> ... and indeed, the code seems to be wrong here. Let's fix this
> by using a bitwise OR instead of logical OR and by indenting
> the code with the right level.
>
> Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> ---
> Please note that this is compile-tested only. I don't have a clue
> about that libcacard stuff, so if you feel confident in this area,
> please have a look whether this change really makes sense.
> ---
>  libcacard/card_7816.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
> index 814fa16..0f7a006 100644
> --- a/libcacard/card_7816.c
> +++ b/libcacard/card_7816.c
> @@ -270,7 +270,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
>              }
>              /* calculate the first extended value. Could be either Le or Lc */
>              Le = (apdu->a_header->ah_body[0] << 8)
> -               || apdu->a_header->ah_body[1];
> +                | apdu->a_header->ah_body[1];
>              if (L == 3) {
>                  /* 2E extended, return data only */
>                  /*   zero maps to 65536 */
> @@ -292,7 +292,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
>              if (L == Le+5) {
>                  /* 4E extended, parameters and return data */
>                  Le = (apdu->a_data[apdu->a_len-2] << 8)
> -                   || apdu->a_data[apdu->a_len-1];
> +                    | apdu->a_data[apdu->a_len-1];
>                  apdu->a_Le = Le ? Le : 65536;
>                  return VCARD7816_STATUS_SUCCESS;
>              }
> @@ -657,7 +657,7 @@ vcard7816_vm_process_apdu(VCard *card, VCardAPDU *apdu,
>                      }
>                  }
>              } else {
> -                    status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
> +                status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
>                  *response = vcard_make_response(status);
>              }
>          }

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch'
  2015-09-14 12:11 ` Markus Armbruster
@ 2015-09-14 12:46   ` Marc-André Lureau
  2015-09-14 13:47     ` Markus Armbruster
  0 siblings, 1 reply; 4+ messages in thread
From: Marc-André Lureau @ 2015-09-14 12:46 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Jeremy White, Thomas Huth, QEMU

Hi

On Mon, Sep 14, 2015 at 2:11 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Stumbled over this while throwing out old mail.  Copying Marc-André and
> Jeremy.
>

Thanks, the patch is applied in my libcacard "next" branch
(https://github.com/elmarco/libcacard/tree/next).

> Thomas Huth <thuth@linux.vnet.ibm.com> writes:
>
>> 'smatch' complains about two bugs and one style issue in card_7816.c:
>>
>> libcacard/card_7816.c:273 vcard_apdu_set_length() warn: should this be a bitwise op?
>> libcacard/card_7816.c:295 vcard_apdu_set_length() warn: should this be a bitwise op?
>> libcacard/card_7816.c:661 vcard7816_vm_process_apdu() warn: inconsistent indenting
>>
>> ... and indeed, the code seems to be wrong here. Let's fix this
>> by using a bitwise OR instead of logical OR and by indenting
>> the code with the right level.
>>
>> Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
>> ---
>> Please note that this is compile-tested only. I don't have a clue
>> about that libcacard stuff, so if you feel confident in this area,
>> please have a look whether this change really makes sense.
>> ---
>>  libcacard/card_7816.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
>> index 814fa16..0f7a006 100644
>> --- a/libcacard/card_7816.c
>> +++ b/libcacard/card_7816.c
>> @@ -270,7 +270,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
>>              }
>>              /* calculate the first extended value. Could be either Le or Lc */
>>              Le = (apdu->a_header->ah_body[0] << 8)
>> -               || apdu->a_header->ah_body[1];
>> +                | apdu->a_header->ah_body[1];
>>              if (L == 3) {
>>                  /* 2E extended, return data only */
>>                  /*   zero maps to 65536 */
>> @@ -292,7 +292,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
>>              if (L == Le+5) {
>>                  /* 4E extended, parameters and return data */
>>                  Le = (apdu->a_data[apdu->a_len-2] << 8)
>> -                   || apdu->a_data[apdu->a_len-1];
>> +                    | apdu->a_data[apdu->a_len-1];
>>                  apdu->a_Le = Le ? Le : 65536;
>>                  return VCARD7816_STATUS_SUCCESS;
>>              }
>> @@ -657,7 +657,7 @@ vcard7816_vm_process_apdu(VCard *card, VCardAPDU *apdu,
>>                      }
>>                  }
>>              } else {
>> -                    status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
>> +                status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
>>                  *response = vcard_make_response(status);
>>              }
>>          }



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch'
  2015-09-14 12:46   ` Marc-André Lureau
@ 2015-09-14 13:47     ` Markus Armbruster
  0 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2015-09-14 13:47 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: Jeremy White, Thomas Huth, QEMU

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Mon, Sep 14, 2015 at 2:11 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> Stumbled over this while throwing out old mail.  Copying Marc-André and
>> Jeremy.
>>
>
> Thanks, the patch is applied in my libcacard "next" branch
> (https://github.com/elmarco/libcacard/tree/next).

Copying Thomas at his current e-mail address.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-09-14 13:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-19 14:53 [Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch' Thomas Huth
2015-09-14 12:11 ` Markus Armbruster
2015-09-14 12:46   ` Marc-André Lureau
2015-09-14 13:47     ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.