Re: [PATCH v2] tag: Implement readout of tag UID via DBus interface
by Frieder Schrempf
Hi Fabian,
On 16.03.21 19:49, Gottstein, Fabian wrote:
> Hi Frieder,
>
> thanks for the patch.
thanks for your feedback.
>
> Could you please also consider the following situation:
> In the case of a NFC Tag Type 1, the identifier is delivered via the RID command (see NFC Digital Protocol). Thus, the Tag's nfcid property is updated in a later step.
> To inform the neard users, a property changed signal has to be emitted when nfcid has changed (in near_tag_set_nfcid). Also, a exists() handler for the new DBus property should be implemented.
I'm new to NFC and D-Bus, so I don't know much about what use-cases and
requirements there are.
Your request sounds reasonable and I think I have a rough understanding
of what is probably needed to implement this. Still to actually do this
I need to look at the specifications and the code more closely and I
don't know if/when I will find time to do this.
Also I don't have any hardware to test this with NFC type 1 tags.
>
> Another thing regarding building the response message:
> The following code snippet could simplify and improve the readability of the usage of the dbus message builder:
>
> dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE_AS_STRING, &entry);
> dbus_message_iter_append_fixed_array(&entry, DBUS_TYPE_BYTE, &uid, len);
> dbus_message_iter_close_container(iter, &entry);
>
Thanks for the improved code, I will use this instead.
Frieder
>
>
> -----Original Message-----
> From: Schrempf Frieder <frieder.schrempf(a)kontron.de>
> Sent: Dienstag, 16. März 2021 12:22
> To: Samuel Ortiz <sameo(a)linux.intel.com>; linux-nfc(a)lists.01.org
> Cc: Frieder Schrempf <frieder.schrempf(a)kontron.de>
> Subject: [linux-nfc] [PATCH v2] tag: Implement readout of tag UID via DBus interface
>
> Caution: This e-mail originated from outside of Philips, be careful for phishing.
>
>
> From: Frieder Schrempf <frieder.schrempf(a)kontron.de>
>
> NFC tags usually provide an unique identifier. Neard already checks if one of the two types of identifiers is available, reads them from tags and stores them in near_tag.nfcid or near_tag.iso15693_uid respectively.
>
> Though currently it is not possible for any client application to get this information via the D-Bus interface as no property for the UID is implemented.
>
> This adds a 'Uid' property to the D-Bus interface for tags, which exposes the UID of the tag as byte array. If nfcid is available this is returned as UID, otherwise if iso15693_uid is available this is returned. If no UID is available, no 'Uid' property is exposed.
>
> Signed-off-by: Frieder Schrempf <frieder.schrempf(a)kontron.de>
> ---
> Changes in v2:
> * Add whitespaces after 'for' statements
> * Add more details to the commit message
> ---
> src/tag.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 54 insertions(+), 3 deletions(-)
>
> diff --git a/src/tag.c b/src/tag.c
> index 9eba4ee..d530893 100644
> --- a/src/tag.c
> +++ b/src/tag.c
> @@ -53,6 +53,7 @@ struct near_tag {
> uint8_t nfcid_len;
>
> uint8_t iso15693_dsfid;
> + uint8_t iso15693_uid_len;
> uint8_t iso15693_uid[NFC_MAX_ISO15693_UID_LEN];
>
> size_t data_length;
> @@ -168,6 +169,29 @@ static const char *type_string(struct near_tag *tag)
> return type;
> }
>
> +static const uint8_t uid_array(struct near_tag *tag, uint8_t **uid) {
> + if (tag->nfcid_len) {
> + DBG("NFCID: ");
> + for (int i = 0; i < tag->nfcid_len; i++)
> + DBG("%x", tag->nfcid[i]);
> +
> + *uid = tag->nfcid;
> +
> + return tag->nfcid_len;
> + } else if (tag->iso15693_uid_len) {
> + DBG("ISO-UID: ");
> + for (int i = 0; i < tag->iso15693_uid_len; i++)
> + DBG("%x", tag->iso15693_uid[i]);
> +
> + *uid = tag->iso15693_uid;
> +
> + return tag->iso15693_uid_len;
> + }
> +
> + return 0;
> +}
> +
> static const char *protocol_string(struct near_tag *tag) {
> const char *protocol;
> @@ -219,6 +243,30 @@ static gboolean property_get_type(const GDBusPropertyTable *property,
> return TRUE;
> }
>
> +static gboolean property_get_uid(const GDBusPropertyTable *property,
> + DBusMessageIter *iter, void
> +*user_data) {
> + struct near_tag *tag = user_data;
> + DBusMessageIter entry;
> + uint8_t *uid;
> + uint8_t len;
> +
> + len = uid_array(tag, &uid);
> + if (!uid || !len)
> + return FALSE;
> +
> + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
> + "{y}", &entry);
> +
> + for (int i = 0; i < len; i++)
> + dbus_message_iter_append_basic(&entry, DBUS_TYPE_BYTE,
> + (void *)&uid[i]);
> +
> + dbus_message_iter_close_container(iter, &entry);
> +
> + return TRUE;
> +}
> +
> static gboolean property_get_protocol(const GDBusPropertyTable *property,
> DBusMessageIter *iter, void *user_data) { @@ -526,6 +574,7 @@ static const GDBusPropertyTable tag_properties[] = {
> { "Protocol", "s", property_get_protocol },
> { "ReadOnly", "b", property_get_readonly },
> { "Adapter", "o", property_get_adapter },
> + { "Uid", "ay", property_get_uid },
>
> { }
> };
> @@ -671,8 +720,10 @@ static int tag_initialize(struct near_tag *tag,
> if (nfcid_len && nfcid_len <= NFC_MAX_NFCID1_LEN) {
> tag->nfcid_len = nfcid_len;
> memcpy(tag->nfcid, nfcid, nfcid_len);
> - } else if (iso15693_uid_len) {
> + } else if (iso15693_uid_len &&
> + iso15693_uid_len <= NFC_MAX_ISO15693_UID_LEN) {
> tag->iso15693_dsfid = iso15693_dsfid;
> + tag->iso15693_uid_len = iso15693_uid_len;
> memcpy(tag->iso15693_uid, iso15693_uid, iso15693_uid_len);
> }
>
> @@ -837,11 +888,11 @@ uint8_t *near_tag_get_iso15693_uid(uint32_t adapter_idx, uint32_t target_idx)
> if (!tag)
> goto fail;
>
> - iso15693_uid = g_try_malloc0(NFC_MAX_ISO15693_UID_LEN);
> + iso15693_uid = g_try_malloc0(tag->iso15693_uid_len);
> if (!iso15693_uid)
> goto fail;
>
> - memcpy(iso15693_uid, tag->iso15693_uid, NFC_MAX_ISO15693_UID_LEN);
> + memcpy(iso15693_uid, tag->iso15693_uid, tag->iso15693_uid_len);
>
> return iso15693_uid;
>
> --
> 2.25.1
> _______________________________________________
> Linux-nfc mailing list -- linux-nfc(a)lists.01.org To unsubscribe send an email to linux-nfc-leave(a)lists.01.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>
> ________________________________
> The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.
>
10 months, 1 week
[neard][PATCH v2 00/73] combined fixes - warnings, memory leaks, memory corruption
by Krzysztof Kozlowski
Hi,
Mark asked for resending fixes separate from setting up CI under Github
Actions, so here is a combination of all my previously sent patches
related only to the actual C code:
https://lore.kernel.org/linux-nfc/20210710033859.3989-1-krzysztof.kozlows...
https://lore.kernel.org/linux-nfc/20210711202102.18094-1-krzysztof.kozlow...
https://lore.kernel.org/linux-nfc/20210714110518.104655-1-krzysztof.kozlo...
https://lore.kernel.org/linux-nfc/20210716100844.51360-1-krzysztof.kozlow...
This includes fixes for GCC/clang warnings, memory leaks, memory
corruption and few other minor fixes. The UTF-8 and UTF-16 is not
entirely fixed and I removed the work-in-progress patches.
Changes since v1 are:
1. Group all patches based on prefix (unit file), instead of topic.
2. Put the build-related patches at the end.
3. Remove all CI-related patches.
4. Drop patch: ndef: fix parsing of UTF-16 text payload.
5. Fix commit msg in: nfctool: pass the format as string literal
Best regards,
Krzysztof
Krzysztof Kozlowski (73):
Drop empty NEWS
nfctool: fix adapter_get_devices() cast-function-type
nfctool: fix adapter_print_target() cast-function-type
nfctool: fix adapter_print_info() cast-function-type
nfctool: fix adapter_compare_idx() cast-function-type
nfctool: fix nfctool_send_dep_link_up() cast-function-type
nfctool: fix nfctool_print_and_remove_snl() cast-function-type
nfctool: use proper format for integers (-Wformat)
nfctool: pass the format as string literal
dbus: fix -Wformat in near_dbus_encode_string()
unit: pass real UTF-8 for testing text NDEF
ndef: check UTF-16 text payload length
ndef: silence clang -Wcast-align warning
ndef: use NDEF_TEXT_RECORD_UTF16_STATUS define
ndef: use proper format for integers (-Wformat)
ndef: make freeing near_ndef_message reusable
se: fix multiple apdu definitions
se: silence clang -Wcast-align warning
se: use proper format for integers (-Wformat)
adapter: adjust indentation of continued arguments
adapter: use proper format for integers (-Wformat)
gdbus: do not shadow global 'pending' variable (-Wshadow)
nciattach: fix poll.h include location
nciattach: do not shadow other local 'opt' variable (-Wshadow)
bluetooth: use proper format for integers (-Wformat)
nfctype2: use proper format for integers (-Wformat)
nfctype3: use proper format for integers (-Wformat)
nfctype5: use proper format for integers (-Wformat)
nfctype5: fix returning uninitialized stack value in
t5_tag_is_ti_pro()
mifare: use proper format for integers (-Wformat)
mifare: use unsigned int to suppress compiler -Wstrict-overflow
p2p: use proper format for integers (-Wformat)
npp: use proper format for integers (-Wformat)
device: use proper format for integers (-Wformat)
manager: use proper format for integers (-Wformat)
netlink: use proper format for integers (-Wformat)
gdbus: annotate printf-like functions as accepting format
snep-send: fix near_ndef_message memory leak
tag: use proper format for integers (-Wformat)
tag: do not open-code freeing ndef message
snep: do not open-code freeing ndef message
snep: remove useless NULL-ify of local pointer variable
snep: fix double free of GSList
snep: fix fragmented response memory leaks
unit: use g_assert_cmpstr()
unit: use g_assert_cmpint() and g_assert_cmpuint()
unit: fix recv() and send() return types
unit: use g_assert_null()
unit: use g_assert_cmpmem()
unit: use proper pointer to uint8_t in test_snep_read_recv_fragments()
unit: do not shadow global 'text' variable (-Wshadow)
unit: do not shadow global 'uri' variable (-Wshadow)
unit: use proper format for integers (-Wformat)
unit: fix memory leaks in test-ndef-parse
unit: do not open-code freeing ndef message
unit: fix memory leaks in test-ndef-build
unit: fix memory leaks in test-snep-read error paths
unit: fix record memory leak in test-snep-read
unit: fix records GList memory leak in test-snep-read
unit: do not pass NULL to memcpy()
unit: do not search for headers locally where they do not exist
unit: remove duplicated invalid definitions in test-snep-read
unit: remove duplicated definitions in test-ndef-parse
unit: add few asserts in test-snep-read
HACKING: refine required packages
build: fix setting CFLAGS on dash shell (Alpine Linux)
build: add more compiler warnings
build: enable -Wshadow and -Wformat-signedness compiler warnings
build: enable -Wformat=2 warnings
build: enable -Wunsafe-loop-optimizations and -Wstrict-overflow=2
warnings
build: fix missing usage of PIE check result
build: add support for GCC sanitizers (asan, lsan and ubsan)
AUTHORS: Mention Krzysztof Kozlowski's contributions
AUTHORS | 1 +
HACKING | 27 ++--
Makefile.am | 2 +-
NEWS | 0
accflags.m4 | 52 +++++++
acinclude.m4 | 44 ++++--
configure.ac | 52 ++++++-
gdbus/gdbus.h | 15 +-
gdbus/object.c | 22 +--
include/ndef.h | 1 +
plugins/mifare.c | 14 +-
plugins/nfctype2.c | 12 +-
plugins/nfctype3.c | 2 +-
plugins/nfctype5.c | 8 +-
plugins/npp.c | 2 +-
plugins/p2p.c | 4 +-
se/ace.c | 64 ++++++---
se/apdu.c | 8 +-
se/plugins/nfc.c | 8 +-
se/plugins/tizen.c | 2 +-
se/se.c | 6 +-
se/seel.h | 2 +-
src/adapter.c | 41 +++---
src/bluetooth.c | 4 +-
src/dbus.c | 2 +-
src/device.c | 6 +-
src/manager.c | 4 +-
src/ndef-private.h | 140 ++++++++++++++++++
src/ndef.c | 217 ++++++----------------------
src/netlink.c | 16 +--
src/snep.c | 14 +-
src/tag.c | 14 +-
tools/nciattach.c | 10 +-
tools/nfctool/adapter.c | 20 +--
tools/nfctool/adapter.h | 2 +-
tools/nfctool/llcp-decode.c | 4 +-
tools/nfctool/main.c | 30 ++--
tools/nfctool/sniffer.c | 10 +-
tools/snep-send.c | 2 +
unit/test-ndef-build.c | 24 ++--
unit/test-ndef-parse.c | 278 +++++++++++-------------------------
unit/test-snep-read.c | 117 ++++++++-------
unit/test-utils.c | 8 --
unit/test-utils.h | 83 +----------
44 files changed, 691 insertions(+), 703 deletions(-)
delete mode 100644 NEWS
create mode 100644 accflags.m4
create mode 100644 src/ndef-private.h
--
2.27.0
10 months, 3 weeks
[PATCH v3] nfc: hci: pass callback data param as pointer in nci_request()
by Krzysztof Kozlowski
The nci_request() receives a callback function and unsigned long data
argument "opt" which is passed to the callback. Almost all of the
nci_request() callers pass pointer to a stack variable as data argument.
Only few pass scalar value (e.g. u8).
All such callbacks do not modify passed data argument and in previous
commit they were made as const. However passing pointers via unsigned
long removes the const annotation. The callback could simply cast
unsigned long to a pointer to writeable memory.
Use "const void *" as type of this "opt" argument to solve this and
prevent modifying the pointed contents. This is also consistent with
generic pattern of passing data arguments - via "void *". In few places
which pass scalar values, use casts via "unsigned long" to suppress any
warnings.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
---
Changes since v2:
1. Cast all constants to (void *) to avoid sparse warning "warning:
Using plain integer as NULL pointer"
---
include/net/nfc/nci_core.h | 4 +-
net/nfc/nci/core.c | 119 ++++++++++++++++++-------------------
net/nfc/nci/hci.c | 16 +++--
3 files changed, 67 insertions(+), 72 deletions(-)
diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 4770a81f4aa7..a964daedc17b 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -276,8 +276,8 @@ int nci_register_device(struct nci_dev *ndev);
void nci_unregister_device(struct nci_dev *ndev);
int nci_request(struct nci_dev *ndev,
void (*req)(struct nci_dev *ndev,
- unsigned long opt),
- unsigned long opt, __u32 timeout);
+ const void *opt),
+ const void *opt, __u32 timeout);
int nci_prop_cmd(struct nci_dev *ndev, __u8 oid, size_t len,
const __u8 *payload);
int nci_core_cmd(struct nci_dev *ndev, __u16 opcode, size_t len,
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 80a5c2a8e9fa..82ab39d80726 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -95,8 +95,8 @@ static void nci_req_cancel(struct nci_dev *ndev, int err)
/* Execute request and wait for completion. */
static int __nci_request(struct nci_dev *ndev,
- void (*req)(struct nci_dev *ndev, unsigned long opt),
- unsigned long opt, __u32 timeout)
+ void (*req)(struct nci_dev *ndev, const void *opt),
+ const void *opt, __u32 timeout)
{
int rc = 0;
long completion_rc;
@@ -139,8 +139,8 @@ static int __nci_request(struct nci_dev *ndev,
inline int nci_request(struct nci_dev *ndev,
void (*req)(struct nci_dev *ndev,
- unsigned long opt),
- unsigned long opt, __u32 timeout)
+ const void *opt),
+ const void *opt, __u32 timeout)
{
int rc;
@@ -155,7 +155,7 @@ inline int nci_request(struct nci_dev *ndev,
return rc;
}
-static void nci_reset_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_reset_req(struct nci_dev *ndev, const void *opt)
{
struct nci_core_reset_cmd cmd;
@@ -163,17 +163,17 @@ static void nci_reset_req(struct nci_dev *ndev, unsigned long opt)
nci_send_cmd(ndev, NCI_OP_CORE_RESET_CMD, 1, &cmd);
}
-static void nci_init_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_init_req(struct nci_dev *ndev, const void *opt)
{
u8 plen = 0;
if (opt)
plen = sizeof(struct nci_core_init_v2_cmd);
- nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, plen, (void *)opt);
+ nci_send_cmd(ndev, NCI_OP_CORE_INIT_CMD, plen, opt);
}
-static void nci_init_complete_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_init_complete_req(struct nci_dev *ndev, const void *opt)
{
struct nci_rf_disc_map_cmd cmd;
struct disc_map_config *cfg = cmd.mapping_configs;
@@ -215,10 +215,9 @@ struct nci_set_config_param {
const __u8 *val;
};
-static void nci_set_config_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_set_config_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_set_config_param *param =
- (struct nci_set_config_param *)opt;
+ const struct nci_set_config_param *param = opt;
struct nci_core_set_config_cmd cmd;
BUG_ON(param->len > NCI_MAX_PARAM_LEN);
@@ -236,10 +235,9 @@ struct nci_rf_discover_param {
__u32 tm_protocols;
};
-static void nci_rf_discover_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_rf_discover_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_rf_discover_param *param =
- (struct nci_rf_discover_param *)opt;
+ const struct nci_rf_discover_param *param = opt;
struct nci_rf_disc_cmd cmd;
cmd.num_disc_configs = 0;
@@ -302,10 +300,9 @@ struct nci_rf_discover_select_param {
__u8 rf_protocol;
};
-static void nci_rf_discover_select_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_rf_discover_select_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_rf_discover_select_param *param =
- (struct nci_rf_discover_select_param *)opt;
+ const struct nci_rf_discover_select_param *param = opt;
struct nci_rf_discover_select_cmd cmd;
cmd.rf_discovery_id = param->rf_discovery_id;
@@ -329,11 +326,11 @@ static void nci_rf_discover_select_req(struct nci_dev *ndev, unsigned long opt)
sizeof(struct nci_rf_discover_select_cmd), &cmd);
}
-static void nci_rf_deactivate_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_rf_deactivate_req(struct nci_dev *ndev, const void *opt)
{
struct nci_rf_deactivate_cmd cmd;
- cmd.type = opt;
+ cmd.type = (unsigned long)opt;
nci_send_cmd(ndev, NCI_OP_RF_DEACTIVATE_CMD,
sizeof(struct nci_rf_deactivate_cmd), &cmd);
@@ -345,10 +342,9 @@ struct nci_cmd_param {
const __u8 *payload;
};
-static void nci_generic_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_generic_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_cmd_param *param =
- (struct nci_cmd_param *)opt;
+ const struct nci_cmd_param *param = opt;
nci_send_cmd(ndev, param->opcode, param->len, param->payload);
}
@@ -361,7 +357,7 @@ int nci_prop_cmd(struct nci_dev *ndev, __u8 oid, size_t len, const __u8 *payload
param.len = len;
param.payload = payload;
- return __nci_request(ndev, nci_generic_req, (unsigned long)¶m,
+ return __nci_request(ndev, nci_generic_req, ¶m,
msecs_to_jiffies(NCI_CMD_TIMEOUT));
}
EXPORT_SYMBOL(nci_prop_cmd);
@@ -375,21 +371,21 @@ int nci_core_cmd(struct nci_dev *ndev, __u16 opcode, size_t len,
param.len = len;
param.payload = payload;
- return __nci_request(ndev, nci_generic_req, (unsigned long)¶m,
+ return __nci_request(ndev, nci_generic_req, ¶m,
msecs_to_jiffies(NCI_CMD_TIMEOUT));
}
EXPORT_SYMBOL(nci_core_cmd);
int nci_core_reset(struct nci_dev *ndev)
{
- return __nci_request(ndev, nci_reset_req, 0,
+ return __nci_request(ndev, nci_reset_req, (void *)0,
msecs_to_jiffies(NCI_RESET_TIMEOUT));
}
EXPORT_SYMBOL(nci_core_reset);
int nci_core_init(struct nci_dev *ndev)
{
- return __nci_request(ndev, nci_init_req, 0,
+ return __nci_request(ndev, nci_init_req, (void *)0,
msecs_to_jiffies(NCI_INIT_TIMEOUT));
}
EXPORT_SYMBOL(nci_core_init);
@@ -399,9 +395,9 @@ struct nci_loopback_data {
struct sk_buff *data;
};
-static void nci_send_data_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_send_data_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_loopback_data *data = (struct nci_loopback_data *)opt;
+ const struct nci_loopback_data *data = opt;
nci_send_data(ndev, data->conn_id, data->data);
}
@@ -462,7 +458,7 @@ int nci_nfcc_loopback(struct nci_dev *ndev, const void *data, size_t data_len,
loopback_data.data = skb;
ndev->cur_conn_id = conn_id;
- r = nci_request(ndev, nci_send_data_req, (unsigned long)&loopback_data,
+ r = nci_request(ndev, nci_send_data_req, &loopback_data,
msecs_to_jiffies(NCI_DATA_TIMEOUT));
if (r == NCI_STATUS_OK && resp)
*resp = conn_info->rx_skb;
@@ -495,7 +491,7 @@ static int nci_open_device(struct nci_dev *ndev)
rc = ndev->ops->init(ndev);
if (!rc) {
- rc = __nci_request(ndev, nci_reset_req, 0,
+ rc = __nci_request(ndev, nci_reset_req, (void *)0,
msecs_to_jiffies(NCI_RESET_TIMEOUT));
}
@@ -508,10 +504,10 @@ static int nci_open_device(struct nci_dev *ndev)
.feature1 = NCI_FEATURE_DISABLE,
.feature2 = NCI_FEATURE_DISABLE
};
- unsigned long opt = 0;
+ const void *opt = NULL;
if (ndev->nci_ver & NCI_VER_2_MASK)
- opt = (unsigned long)&nci_init_v2_cmd;
+ opt = &nci_init_v2_cmd;
rc = __nci_request(ndev, nci_init_req, opt,
msecs_to_jiffies(NCI_INIT_TIMEOUT));
@@ -521,7 +517,7 @@ static int nci_open_device(struct nci_dev *ndev)
rc = ndev->ops->post_setup(ndev);
if (!rc) {
- rc = __nci_request(ndev, nci_init_complete_req, 0,
+ rc = __nci_request(ndev, nci_init_complete_req, (void *)0,
msecs_to_jiffies(NCI_INIT_TIMEOUT));
}
@@ -571,7 +567,7 @@ static int nci_close_device(struct nci_dev *ndev)
atomic_set(&ndev->cmd_cnt, 1);
set_bit(NCI_INIT, &ndev->flags);
- __nci_request(ndev, nci_reset_req, 0,
+ __nci_request(ndev, nci_reset_req, (void *)0,
msecs_to_jiffies(NCI_RESET_TIMEOUT));
/* After this point our queues are empty
@@ -637,15 +633,15 @@ int nci_set_config(struct nci_dev *ndev, __u8 id, size_t len, const __u8 *val)
param.len = len;
param.val = val;
- return __nci_request(ndev, nci_set_config_req, (unsigned long)¶m,
+ return __nci_request(ndev, nci_set_config_req, ¶m,
msecs_to_jiffies(NCI_SET_CONFIG_TIMEOUT));
}
EXPORT_SYMBOL(nci_set_config);
-static void nci_nfcee_discover_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_nfcee_discover_req(struct nci_dev *ndev, const void *opt)
{
struct nci_nfcee_discover_cmd cmd;
- __u8 action = opt;
+ __u8 action = (unsigned long)opt;
cmd.discovery_action = action;
@@ -654,15 +650,16 @@ static void nci_nfcee_discover_req(struct nci_dev *ndev, unsigned long opt)
int nci_nfcee_discover(struct nci_dev *ndev, u8 action)
{
- return __nci_request(ndev, nci_nfcee_discover_req, action,
+ unsigned long opt = action;
+
+ return __nci_request(ndev, nci_nfcee_discover_req, (void *)opt,
msecs_to_jiffies(NCI_CMD_TIMEOUT));
}
EXPORT_SYMBOL(nci_nfcee_discover);
-static void nci_nfcee_mode_set_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_nfcee_mode_set_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_nfcee_mode_set_cmd *cmd =
- (struct nci_nfcee_mode_set_cmd *)opt;
+ const struct nci_nfcee_mode_set_cmd *cmd = opt;
nci_send_cmd(ndev, NCI_OP_NFCEE_MODE_SET_CMD,
sizeof(struct nci_nfcee_mode_set_cmd), cmd);
@@ -675,16 +672,14 @@ int nci_nfcee_mode_set(struct nci_dev *ndev, u8 nfcee_id, u8 nfcee_mode)
cmd.nfcee_id = nfcee_id;
cmd.nfcee_mode = nfcee_mode;
- return __nci_request(ndev, nci_nfcee_mode_set_req,
- (unsigned long)&cmd,
+ return __nci_request(ndev, nci_nfcee_mode_set_req, &cmd,
msecs_to_jiffies(NCI_CMD_TIMEOUT));
}
EXPORT_SYMBOL(nci_nfcee_mode_set);
-static void nci_core_conn_create_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_core_conn_create_req(struct nci_dev *ndev, const void *opt)
{
- const struct core_conn_create_data *data =
- (struct core_conn_create_data *)opt;
+ const struct core_conn_create_data *data = opt;
nci_send_cmd(ndev, NCI_OP_CORE_CONN_CREATE_CMD, data->length, data->cmd);
}
@@ -721,24 +716,26 @@ int nci_core_conn_create(struct nci_dev *ndev, u8 destination_type,
}
ndev->cur_dest_type = destination_type;
- r = __nci_request(ndev, nci_core_conn_create_req, (unsigned long)&data,
+ r = __nci_request(ndev, nci_core_conn_create_req, &data,
msecs_to_jiffies(NCI_CMD_TIMEOUT));
kfree(cmd);
return r;
}
EXPORT_SYMBOL(nci_core_conn_create);
-static void nci_core_conn_close_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_core_conn_close_req(struct nci_dev *ndev, const void *opt)
{
- __u8 conn_id = opt;
+ __u8 conn_id = (unsigned long)opt;
nci_send_cmd(ndev, NCI_OP_CORE_CONN_CLOSE_CMD, 1, &conn_id);
}
int nci_core_conn_close(struct nci_dev *ndev, u8 conn_id)
{
+ unsigned long opt = conn_id;
+
ndev->cur_conn_id = conn_id;
- return __nci_request(ndev, nci_core_conn_close_req, conn_id,
+ return __nci_request(ndev, nci_core_conn_close_req, (void *)opt,
msecs_to_jiffies(NCI_CMD_TIMEOUT));
}
EXPORT_SYMBOL(nci_core_conn_close);
@@ -758,14 +755,14 @@ static int nci_set_local_general_bytes(struct nfc_dev *nfc_dev)
param.id = NCI_PN_ATR_REQ_GEN_BYTES;
- rc = nci_request(ndev, nci_set_config_req, (unsigned long)¶m,
+ rc = nci_request(ndev, nci_set_config_req, ¶m,
msecs_to_jiffies(NCI_SET_CONFIG_TIMEOUT));
if (rc)
return rc;
param.id = NCI_LN_ATR_RES_GEN_BYTES;
- return nci_request(ndev, nci_set_config_req, (unsigned long)¶m,
+ return nci_request(ndev, nci_set_config_req, ¶m,
msecs_to_jiffies(NCI_SET_CONFIG_TIMEOUT));
}
@@ -815,7 +812,7 @@ static int nci_start_poll(struct nfc_dev *nfc_dev,
pr_debug("target active or w4 select, implicitly deactivate\n");
rc = nci_request(ndev, nci_rf_deactivate_req,
- NCI_DEACTIVATE_TYPE_IDLE_MODE,
+ (void *)NCI_DEACTIVATE_TYPE_IDLE_MODE,
msecs_to_jiffies(NCI_RF_DEACTIVATE_TIMEOUT));
if (rc)
return -EBUSY;
@@ -837,7 +834,7 @@ static int nci_start_poll(struct nfc_dev *nfc_dev,
param.im_protocols = im_protocols;
param.tm_protocols = tm_protocols;
- rc = nci_request(ndev, nci_rf_discover_req, (unsigned long)¶m,
+ rc = nci_request(ndev, nci_rf_discover_req, ¶m,
msecs_to_jiffies(NCI_RF_DISC_TIMEOUT));
if (!rc)
@@ -856,7 +853,8 @@ static void nci_stop_poll(struct nfc_dev *nfc_dev)
return;
}
- nci_request(ndev, nci_rf_deactivate_req, NCI_DEACTIVATE_TYPE_IDLE_MODE,
+ nci_request(ndev, nci_rf_deactivate_req,
+ (void *)NCI_DEACTIVATE_TYPE_IDLE_MODE,
msecs_to_jiffies(NCI_RF_DEACTIVATE_TIMEOUT));
}
@@ -915,8 +913,7 @@ static int nci_activate_target(struct nfc_dev *nfc_dev,
else
param.rf_protocol = NCI_RF_PROTOCOL_NFC_DEP;
- rc = nci_request(ndev, nci_rf_discover_select_req,
- (unsigned long)¶m,
+ rc = nci_request(ndev, nci_rf_discover_select_req, ¶m,
msecs_to_jiffies(NCI_RF_DISC_SELECT_TIMEOUT));
}
@@ -931,7 +928,7 @@ static void nci_deactivate_target(struct nfc_dev *nfc_dev,
__u8 mode)
{
struct nci_dev *ndev = nfc_get_drvdata(nfc_dev);
- u8 nci_mode = NCI_DEACTIVATE_TYPE_IDLE_MODE;
+ unsigned long nci_mode = NCI_DEACTIVATE_TYPE_IDLE_MODE;
pr_debug("entry\n");
@@ -949,7 +946,7 @@ static void nci_deactivate_target(struct nfc_dev *nfc_dev,
}
if (atomic_read(&ndev->state) == NCI_POLL_ACTIVE) {
- nci_request(ndev, nci_rf_deactivate_req, nci_mode,
+ nci_request(ndev, nci_rf_deactivate_req, (void *)nci_mode,
msecs_to_jiffies(NCI_RF_DEACTIVATE_TIMEOUT));
}
}
@@ -987,8 +984,8 @@ static int nci_dep_link_down(struct nfc_dev *nfc_dev)
} else {
if (atomic_read(&ndev->state) == NCI_LISTEN_ACTIVE ||
atomic_read(&ndev->state) == NCI_DISCOVERY) {
- nci_request(ndev, nci_rf_deactivate_req, 0,
- msecs_to_jiffies(NCI_RF_DEACTIVATE_TIMEOUT));
+ nci_request(ndev, nci_rf_deactivate_req, (void *)0,
+ msecs_to_jiffies(NCI_RF_DEACTIVATE_TIMEOUT));
}
rc = nfc_tm_deactivated(nfc_dev);
diff --git a/net/nfc/nci/hci.c b/net/nfc/nci/hci.c
index a8ff794a8084..e199912ee1e5 100644
--- a/net/nfc/nci/hci.c
+++ b/net/nfc/nci/hci.c
@@ -195,9 +195,9 @@ static int nci_hci_send_data(struct nci_dev *ndev, u8 pipe,
return i;
}
-static void nci_hci_send_data_req(struct nci_dev *ndev, unsigned long opt)
+static void nci_hci_send_data_req(struct nci_dev *ndev, const void *opt)
{
- const struct nci_data *data = (struct nci_data *)opt;
+ const struct nci_data *data = opt;
nci_hci_send_data(ndev, data->pipe, data->cmd,
data->data, data->data_len);
@@ -240,7 +240,7 @@ int nci_hci_send_cmd(struct nci_dev *ndev, u8 gate, u8 cmd,
data.data = param;
data.data_len = param_len;
- r = nci_request(ndev, nci_hci_send_data_req, (unsigned long)&data,
+ r = nci_request(ndev, nci_hci_send_data_req, &data,
msecs_to_jiffies(NCI_DATA_TIMEOUT));
if (r == NCI_STATUS_OK) {
message = (struct nci_hcp_message *)conn_info->rx_skb->data;
@@ -511,9 +511,8 @@ int nci_hci_open_pipe(struct nci_dev *ndev, u8 pipe)
data.data = NULL;
data.data_len = 0;
- return nci_request(ndev, nci_hci_send_data_req,
- (unsigned long)&data,
- msecs_to_jiffies(NCI_DATA_TIMEOUT));
+ return nci_request(ndev, nci_hci_send_data_req, &data,
+ msecs_to_jiffies(NCI_DATA_TIMEOUT));
}
EXPORT_SYMBOL(nci_hci_open_pipe);
@@ -587,8 +586,7 @@ int nci_hci_set_param(struct nci_dev *ndev, u8 gate, u8 idx,
data.data = tmp;
data.data_len = param_len + 1;
- r = nci_request(ndev, nci_hci_send_data_req,
- (unsigned long)&data,
+ r = nci_request(ndev, nci_hci_send_data_req, &data,
msecs_to_jiffies(NCI_DATA_TIMEOUT));
if (r == NCI_STATUS_OK) {
message = (struct nci_hcp_message *)conn_info->rx_skb->data;
@@ -627,7 +625,7 @@ int nci_hci_get_param(struct nci_dev *ndev, u8 gate, u8 idx,
data.data = &idx;
data.data_len = 1;
- r = nci_request(ndev, nci_hci_send_data_req, (unsigned long)&data,
+ r = nci_request(ndev, nci_hci_send_data_req, &data,
msecs_to_jiffies(NCI_DATA_TIMEOUT));
if (r == NCI_STATUS_OK) {
--
2.27.0
11 months
[PATCH v3 0/7] nfc: constify pointed data - missed part
by Krzysztof Kozlowski
Hi,
This was previously sent [1] but got lost. It was a prerequisite to part two of NFC const [2].
Changes since v2:
1. Drop patch previously 7/8 which cases new warnings "warning: Using
plain integer as NULL pointer".
Changes since v1:
1. Add patch 1/8 fixing up nfcmrvl_spi_parse_dt()
[1] https://lore.kernel.org/lkml/20210726145224.146006-1-krzysztof.kozlowski@...
[2] https://lore.kernel.org/linux-nfc/20210729104022.47761-1-krzysztof.kozlow...
Best regards,
Krzysztof
Krzysztof Kozlowski (7):
nfc: mrvl: correct nfcmrvl_spi_parse_dt() device_node argument
nfc: annotate af_nfc_exit() as __exit
nfc: hci: annotate nfc_llc_init() as __init
nfc: constify several pointers to u8, char and sk_buff
nfc: constify local pointer variables
nfc: nci: constify several pointers to u8, sk_buff and other structs
nfc: hci: cleanup unneeded spaces
drivers/nfc/nfcmrvl/spi.c | 2 +-
drivers/nfc/pn544/pn544.c | 4 +-
include/net/nfc/nci_core.h | 14 +++---
include/net/nfc/nfc.h | 4 +-
net/nfc/af_nfc.c | 2 +-
net/nfc/core.c | 6 +--
net/nfc/hci/core.c | 8 ++--
net/nfc/hci/llc.c | 2 +-
net/nfc/hci/llc_shdlc.c | 10 ++---
net/nfc/llcp.h | 8 ++--
net/nfc/llcp_commands.c | 46 +++++++++++---------
net/nfc/llcp_core.c | 44 ++++++++++---------
net/nfc/nci/core.c | 48 +++++++++++----------
net/nfc/nci/data.c | 12 +++---
net/nfc/nci/hci.c | 38 ++++++++---------
net/nfc/nci/ntf.c | 87 ++++++++++++++++++++------------------
net/nfc/nci/rsp.c | 48 +++++++++++----------
net/nfc/nci/spi.c | 2 +-
net/nfc/netlink.c | 2 +-
net/nfc/nfc.h | 2 +-
20 files changed, 206 insertions(+), 183 deletions(-)
--
2.27.0
11 months
Re: [PATCH v2 7/8] nfc: hci: pass callback data param as pointer in nci_request()
by Krzysztof Kozlowski
On 30/07/2021 15:58, Jakub Kicinski wrote:
> On Fri, 30 Jul 2021 15:56:19 +0200 Krzysztof Kozlowski wrote:
>> On 30/07/2021 15:49, Jakub Kicinski wrote:
>>> This generates a bunch of warnings:
>>>
>>> net/nfc/nci/core.c:381:51: warning: Using plain integer as NULL pointer
>>> net/nfc/nci/core.c:388:50: warning: Using plain integer as NULL pointer
>>> net/nfc/nci/core.c:494:57: warning: Using plain integer as NULL pointer
>>> net/nfc/nci/core.c:520:65: warning: Using plain integer as NULL pointer
>>> net/nfc/nci/core.c:570:44: warning: Using plain integer as NULL pointer
>>> net/nfc/nci/core.c:815:34: warning: Using plain integer as NULL pointer
>>> net/nfc/nci/core.c:856:50: warning: Using plain integer as NULL pointer
>>
>> Indeed. Not that code before was better - the logic was exactly the
>> same. I might think more how to avoid these and maybe pass pointer to
>> stack value (like in other cases).
>>
>> The 7/8 and 8/8 could be skipped in such case.
>
> We don't usually take parts of series, would you mind resending first 6
> or respinning with the warnings addressed?
Sure, no problem.
Best regards,
Krzysztof
11 months
Re: [PATCH v2 7/8] nfc: hci: pass callback data param as pointer in nci_request()
by Krzysztof Kozlowski
On 30/07/2021 15:49, Jakub Kicinski wrote:
> On Fri, 30 Jul 2021 08:56:24 +0200 Krzysztof Kozlowski wrote:
>> The nci_request() receives a callback function and unsigned long data
>> argument "opt" which is passed to the callback. Almost all of the
>> nci_request() callers pass pointer to a stack variable as data argument.
>> Only few pass scalar value (e.g. u8).
>>
>> All such callbacks do not modify passed data argument and in previous
>> commit they were made as const. However passing pointers via unsigned
>> long removes the const annotation. The callback could simply cast
>> unsigned long to a pointer to writeable memory.
>>
>> Use "const void *" as type of this "opt" argument to solve this and
>> prevent modifying the pointed contents. This is also consistent with
>> generic pattern of passing data arguments - via "void *". In few places
>> passing scalar values, use casts via "unsigned long" to suppress any
>> warnings.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)canonical.com>
>
> This generates a bunch of warnings:
>
> net/nfc/nci/core.c:381:51: warning: Using plain integer as NULL pointer
> net/nfc/nci/core.c:388:50: warning: Using plain integer as NULL pointer
> net/nfc/nci/core.c:494:57: warning: Using plain integer as NULL pointer
> net/nfc/nci/core.c:520:65: warning: Using plain integer as NULL pointer
> net/nfc/nci/core.c:570:44: warning: Using plain integer as NULL pointer
> net/nfc/nci/core.c:815:34: warning: Using plain integer as NULL pointer
> net/nfc/nci/core.c:856:50: warning: Using plain integer as NULL pointer
Indeed. Not that code before was better - the logic was exactly the
same. I might think more how to avoid these and maybe pass pointer to
stack value (like in other cases).
The 7/8 and 8/8 could be skipped in such case.
>
> BTW applying this set will resolve the warnings introduced by applying
> "part 2" out of order, right? No further action needed?
Yes, it will resolve all warnings. No further action needed, at least I
am not aware of any new issues.
Best regards,
Krzysztof
11 months
[PATCH v2 0/8] nfc: constify pointed data - missed part
by Krzysztof Kozlowski
Hi,
This was previously sent [1] but got lost. It was a prerequisite to part two of NFC const [2].
Changes since v1:
1. Add patch 1/8 fixing up nfcmrvl_spi_parse_dt()
[1] https://lore.kernel.org/lkml/20210726145224.146006-1-krzysztof.kozlowski@...
[2] https://lore.kernel.org/linux-nfc/20210729104022.47761-1-krzysztof.kozlow...
Best regards,
Krzysztof
Krzysztof Kozlowski (8):
nfc: mrvl: correct nfcmrvl_spi_parse_dt() device_node argument
nfc: annotate af_nfc_exit() as __exit
nfc: hci: annotate nfc_llc_init() as __init
nfc: constify several pointers to u8, char and sk_buff
nfc: constify local pointer variables
nfc: nci: constify several pointers to u8, sk_buff and other structs
nfc: hci: pass callback data param as pointer in nci_request()
nfc: hci: cleanup unneeded spaces
drivers/nfc/nfcmrvl/spi.c | 2 +-
drivers/nfc/pn544/pn544.c | 4 +-
include/net/nfc/nci_core.h | 18 ++---
include/net/nfc/nfc.h | 4 +-
net/nfc/af_nfc.c | 2 +-
net/nfc/core.c | 6 +-
net/nfc/hci/core.c | 8 +--
net/nfc/hci/llc.c | 2 +-
net/nfc/hci/llc_shdlc.c | 10 +--
net/nfc/llcp.h | 8 +--
net/nfc/llcp_commands.c | 46 +++++++------
net/nfc/llcp_core.c | 44 ++++++------
net/nfc/nci/core.c | 134 ++++++++++++++++++-------------------
net/nfc/nci/data.c | 12 ++--
net/nfc/nci/hci.c | 52 +++++++-------
net/nfc/nci/ntf.c | 87 +++++++++++++-----------
net/nfc/nci/rsp.c | 48 +++++++------
net/nfc/nci/spi.c | 2 +-
net/nfc/netlink.c | 2 +-
net/nfc/nfc.h | 2 +-
20 files changed, 255 insertions(+), 238 deletions(-)
--
2.27.0
11 months
[PATCH 00/12] nfc: constify, continued (part 2)
by Krzysztof Kozlowski
Hi,
On top of:
nfc: constify pointed data
https://lore.kernel.org/lkml/20210726145224.146006-1-krzysztof.kozlowski@...
Best regards,
Krzysztof
Krzysztof Kozlowski (12):
nfc: constify passed nfc_dev
nfc: mei_phy: constify buffer passed to mei_nfc_send()
nfc: port100: constify several pointers
nfc: trf7970a: constify several pointers
nfc: virtual_ncidev: constify pointer to nfc_dev
nfc: nfcsim: constify drvdata (struct nfcsim)
nfc: fdp: drop unneeded cast for printing firmware size in dev_dbg()
nfc: fdp: use unsigned int as loop iterator
nfc: fdp: constify several pointers
nfc: microread: constify several pointers
nfc: mrvl: constify several pointers
nfc: mrvl: constify static nfcmrvl_if_ops
drivers/nfc/fdp/fdp.c | 27 +++++++++++-----------
drivers/nfc/fdp/fdp.h | 2 +-
drivers/nfc/fdp/i2c.c | 6 ++---
drivers/nfc/mei_phy.c | 2 +-
drivers/nfc/microread/i2c.c | 2 +-
drivers/nfc/microread/microread.c | 4 ++--
drivers/nfc/microread/microread.h | 2 +-
drivers/nfc/nfcmrvl/fw_dnld.c | 16 +++++++------
drivers/nfc/nfcmrvl/i2c.c | 4 ++--
drivers/nfc/nfcmrvl/main.c | 4 ++--
drivers/nfc/nfcmrvl/nfcmrvl.h | 6 ++---
drivers/nfc/nfcmrvl/spi.c | 6 ++---
drivers/nfc/nfcmrvl/uart.c | 4 ++--
drivers/nfc/nfcmrvl/usb.c | 2 +-
drivers/nfc/nfcsim.c | 2 +-
drivers/nfc/port100.c | 37 +++++++++++++++++--------------
drivers/nfc/trf7970a.c | 17 +++++++-------
drivers/nfc/virtual_ncidev.c | 2 +-
include/net/nfc/nfc.h | 4 ++--
19 files changed, 78 insertions(+), 71 deletions(-)
--
2.27.0
11 months
About NFC Tag emulation on Linux
by Vincas Dargis
Hi list,
I see there's "test-device", "test-tag" helpers in neard-tools package
in Debian, but it looks like it needs actual devices/tags to test
with..?
Is there a way to emulate NFC tag, to "emit" NFC tag discovery events
and NDEF mesages on Linux? Qt supports NFC on Linux via neard daemon,
so I thought it would be very useful to emulate NFC tags (with various
payloads) for development and (automated even?) testing.
Thanks!
11 months, 1 week
