Bug 245739 - [Patch] Improve hccontrol le_read_local_supported_features
Summary: [Patch] Improve hccontrol le_read_local_supported_features
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Hans Petter Selasky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-19 12:07 UTC by Marc Veldman
Modified: 2020-05-18 08:53 UTC (History)
2 users (show)

See Also:


Attachments
Show le features, behave more like read_local_features (5.35 KB, patch)
2020-04-19 12:07 UTC, Marc Veldman
no flags Details | Diff
Improved patch union instead of bitshifting (5.38 KB, patch)
2020-04-19 13:26 UTC, Marc Veldman
no flags Details | Diff
Improved patch: Union with correct types (5.36 KB, patch)
2020-04-19 13:40 UTC, Marc Veldman
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Veldman 2020-04-19 12:07:24 UTC
Created attachment 213559 [details]
Show le features, behave more like read_local_features

Improve hccontrol le_read_local_supported_features
- Actually show the features instead of the binary values.
- Behave like read_local_supported_features

Was:
root@devnovo:/usr/src/usr.sbin/bluetooth/hccontrol # hccontrol -n ubt0hci le_read_local_supported_features
LOCAL SUPPORTED: 0 0 1

Is:
root@devnovo:/usr/src # hccontrol -n ubt0hci le_read_local_supported_features
LE Features:  0x1 00 00 00 00 00 00 00
<LE Encryption>
Comment 1 Hans Petter Selasky freebsd_committer freebsd_triage 2020-04-19 12:34:19 UTC
Hi,

Can you explain what this code is doing?

    		le_features[i] = (uint8_t)(((uintmax_t)rp.le_features >> 8*i) & 0xFF);

Is this code endian / platform safe?

--HPS
Comment 2 Hans Petter Selasky freebsd_committer freebsd_triage 2020-04-19 12:37:04 UTC
What I wanted to say:

Can you re-write this code to use byte access instead of uintmax_t ?

You can use a nameless union like this:

union {
   xxx le_features
   uint8_t le_features_raw[8];
};


le_features[i] = (uint8_t)(((uintmax_t)rp.le_features >> 8*i) & 0xFF);

--HPS
Comment 3 Marc Veldman 2020-04-19 12:52:22 UTC
(In reply to Hans Petter Selasky from comment #2)

Thanks for the suggestio! I will. I was not sure if a union was the right
way to go.

I do wonder however why

typedef struct {
        u_int8_t        status; /*status*/
        u_int64_t       le_features;
} __attribute__ ((packed)) ng_hci_le_read_local_supported_features_rp;

typedef struct {
        u_int8_t        status;                         /* 0x00 - success */
        u_int16_t       con_handle;                     /* Connection handle */
        u_int8_t        features[NG_HCI_FEATURES_SIZE]; /* LMP features bitmsk*/
} __attribute__ ((packed)) ng_hci_read_remote_features_compl_ep;

features is a u_int64_t in the one, and u_int8t[] in the other, since both
are the same (8 octets) in the bluetooth specs.
Comment 4 Marc Veldman 2020-04-19 13:26:38 UTC
Created attachment 213562 [details]
Improved patch union instead of bitshifting

Changed the patch to use a union type instead of bit shifting.
Comment 5 Hans Petter Selasky freebsd_committer freebsd_triage 2020-04-19 13:31:04 UTC
Please use uint64_t instead of uintmax_t.

And remove cast here:

le_features.raw = (uintmax_t) rp.le_features;

--HPS
Comment 6 Marc Veldman 2020-04-19 13:40:38 UTC
Created attachment 213564 [details]
Improved patch: Union with correct types

Done. Apologies for the noise and thanks for the quick responses!
Comment 7 commit-hook freebsd_committer freebsd_triage 2020-04-19 14:23:18 UTC
A commit references this bug:

Author: hselasky
Date: Sun Apr 19 14:22:22 UTC 2020
New revision: 360094
URL: https://svnweb.freebsd.org/changeset/base/360094

Log:
  Improve printing of le features in hccontrol(8).

  Submitted by:	Marc Veldman <marc@bumblingdork.com>
  PR:		245739
  MFC after:	1 week
  Sponsored by:	Mellanox Technologies

Changes:
  head/usr.sbin/bluetooth/hccontrol/hccontrol.h
  head/usr.sbin/bluetooth/hccontrol/le.c
  head/usr.sbin/bluetooth/hccontrol/util.c
Comment 8 commit-hook freebsd_committer freebsd_triage 2020-04-19 14:26:20 UTC
A commit references this bug:

Author: hselasky
Date: Sun Apr 19 14:25:56 UTC 2020
New revision: 360095
URL: https://svnweb.freebsd.org/changeset/base/360095

Log:
  Fix cut and paste off-by-one error in hccontrol(8).
  Make sure strncpy() doesn't write beyond its given buffer.

  PR:		245739
  MFC after:	1 week
  Sponsored by:	Mellanox Technologies

Changes:
  head/usr.sbin/bluetooth/hccontrol/util.c
Comment 9 Hans Petter Selasky freebsd_committer freebsd_triage 2020-04-19 14:26:26 UTC
Submitted with some minor modifications.

Please test!

Thank you!
Comment 10 commit-hook freebsd_committer freebsd_triage 2020-05-18 08:46:32 UTC
A commit references this bug:

Author: hselasky
Date: Mon May 18 08:46:18 UTC 2020
New revision: 361158
URL: https://svnweb.freebsd.org/changeset/base/361158

Log:
  MFC r360094:
  Improve printing of le features in hccontrol(8).

  Submitted by:	Marc Veldman <marc@bumblingdork.com>
  PR:		245739
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/12/
  stable/12/usr.sbin/bluetooth/hccontrol/hccontrol.h
  stable/12/usr.sbin/bluetooth/hccontrol/le.c
  stable/12/usr.sbin/bluetooth/hccontrol/util.c
Comment 11 commit-hook freebsd_committer freebsd_triage 2020-05-18 08:47:33 UTC
A commit references this bug:

Author: hselasky
Date: Mon May 18 08:46:53 UTC 2020
New revision: 361159
URL: https://svnweb.freebsd.org/changeset/base/361159

Log:
  MFC r360094:
  Improve printing of le features in hccontrol(8).

  Submitted by:	Marc Veldman <marc@bumblingdork.com>
  PR:		245739
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/11/
  stable/11/usr.sbin/bluetooth/hccontrol/hccontrol.h
  stable/11/usr.sbin/bluetooth/hccontrol/le.c
  stable/11/usr.sbin/bluetooth/hccontrol/util.c
Comment 12 commit-hook freebsd_committer freebsd_triage 2020-05-18 08:52:38 UTC
A commit references this bug:

Author: hselasky
Date: Mon May 18 08:52:07 UTC 2020
New revision: 361160
URL: https://svnweb.freebsd.org/changeset/base/361160

Log:
  MFC r360095:
  Fix cut and paste off-by-one error in hccontrol(8).
  Make sure strncpy() doesn't write beyond its given buffer.

  PR:		245739
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/12/
  stable/12/usr.sbin/bluetooth/hccontrol/util.c
Comment 13 commit-hook freebsd_committer freebsd_triage 2020-05-18 08:53:39 UTC
A commit references this bug:

Author: hselasky
Date: Mon May 18 08:52:41 UTC 2020
New revision: 361161
URL: https://svnweb.freebsd.org/changeset/base/361161

Log:
  MFC r360095:
  Fix cut and paste off-by-one error in hccontrol(8).
  Make sure strncpy() doesn't write beyond its given buffer.

  PR:		245739
  Sponsored by:	Mellanox Technologies

Changes:
_U  stable/11/
  stable/11/usr.sbin/bluetooth/hccontrol/util.c