Search Linux Wireless

Re: [PATCH 2/6] NFC: add nfc generic netlink interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2011-06-02 at 18:46 -0300, Lauro Ramos Venancio wrote:

> + * @NFC_ATTR_TARGETS: array of targets (see enum nfc_target_attr)


> +static int nfc_genl_msg_put_target(struct sk_buff *msg,
> +				struct nfc_target *target)
> +{
> +	NLA_PUT_U32(msg, NFC_TARGET_ATTR_TARGET_INDEX, target->idx);
> +	NLA_PUT_U32(msg, NFC_TARGET_ATTR_SUPPORTED_PROTOCOLS,
> +				target->supported_protocols);
> +	NLA_PUT_U8(msg, NFC_TARGET_ATTR_SENS_RES, target->sens_res);
> +	NLA_PUT_U8(msg, NFC_TARGET_ATTR_SEL_RES, target->sel_res);
> +
> +	return 0;
> +
> +nla_put_failure:
> +	return -EMSGSIZE;
> +}
> +
> +int nfc_genl_targets_found(struct nfc_dev *dev, struct nfc_target *targets,
> +								int ntargets)
> +{
> +	struct sk_buff *msg;
> +	void *hdr;
> +	struct nlattr *targets_attr;
> +	int i;
> +
> +	pr_debug("%s\n", __func__);
> +
> +	dev->genl_data.poll_req_pid = 0;
> +
> +	msg = nlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	hdr = genlmsg_put(msg, 0, 0, &nfc_genl_family, 0,
> +				NFC_EVENT_TARGETS_FOUND);
> +	if (!hdr)
> +		goto free_msg;
> +
> +	NLA_PUT_U32(msg, NFC_ATTR_DEVICE_INDEX, dev->idx);
> +
> +	targets_attr = nla_nest_start(msg, NFC_ATTR_TARGETS);
> +
> +	for (i = 0; i < ntargets; i++) {
> +		struct nlattr *target = nla_nest_start(msg, i);
> +
> +		if (nfc_genl_msg_put_target(msg, &targets[i]))
> +			goto nla_put_failure;
> +
> +		nla_nest_end(msg, target);
> +	}
> +
> +	nla_nest_end(msg, targets_attr);
> +	genlmsg_end(msg, hdr);
> +
> +	return genlmsg_multicast(msg, 0, nfc_genl_event_mcgrp.id, GFP_ATOMIC);

This is almost certainly not a good idea.

Eventually, you will want to add more information about a target. I know
NFC is a pretty limited protocol, but I wouldn't want to make this
harder in the future than it must be. The way you're doing it, you're
currently limiting yourself to about 100 targets. Each new target
attribute that you might add in the future will reduce that number
significantly.

IMHO, the better way to structure this would be to create an event that
contains no information, and then allow a dump of the targets (with a
generation counter). That way, there are no such artificial limits due
to message sizes.


> +static int nfc_genl_dump_devices(struct sk_buff *skb,
> +                               struct netlink_callback *cb)

You should have a generation counter here so that applications getting a
dump can know whether their dump was a complete and consistent snapshot.
Otherwise, if devices are added or removed during the dump applications
will not be able to know that their dump wasn't right.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux