timmerk / nfc-tools

Automatically exported from code.google.com/p/nfc-tools
0 stars 0 forks source link

r247 is maybe not a solution to... #22

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I have commit r247 in order to respond to one problem:
using freefare_get_tags(), we have an array of all detected (and
supported) tags.
In one of our programs, we use it in a loop to detect entered/leaving
field, consequently an object (this program is object-oriented) is
created/deleted.
When we create an object, we have to keep a MifareTag in order to be able
to reuse it for futur users manipulations but even if we keep a MifareTag
we can't reuse it due to nature of MifareTag: its a pointer on a "hidden"
struct and because we need to freefare_free_tags() (to prevent for
leaking) holded MifareTag is free too...
So a simple (and dirty?) solution is to be able to duplicate tag struct
(like strdup do for strings) and free only one tag:
freefare_duplicate_tag() and freefare_free_tag() was born.

I'm not sure its an real issue but a review is wanted.

Original issue reported on code.google.com by romu...@libnfc.org on 20 Apr 2010 at 3:35

GoogleCodeExporter commented 9 years ago

Original comment by romu...@libnfc.org on 20 Apr 2010 at 3:35

GoogleCodeExporter commented 9 years ago

Hum... I'm not found of the idea of cloning tags: some fields are stateful and 
this way, API allows the user to shoot himself in the foot:

tag2 <- freefare_duplicate_tag (tag1);
mifare_classic_connect (tag1);
mifare_classic_authenticate (tag2); // Fail: tag2 is not connected
mifare_classic_connect (tag2); // May pass... I am not testing while writing 
this.
etc.

Since the idea is to "refresh" a list of known tags, I can see two other way of 
reaching your goal:

1. Extending the freefare_get_tags() function to accept a new (possibly NULL) 
argument `known_tags' and that makes the function basically act as realloc():

  // Get the list of tags for the first time
  tags = freefare_get_tags (device, NULL);
  if (!tags)
    err (1, "Can't get tags list");

  // Update the list
  old_tags = tags;
  tags = freefare_get_tags (device, tags);
  if (!tags) {
    warn ("Can't update tags list");
    tags = old_tags;
  }

2. Add new API functions that:
  - return new tags (e.g. MifareTag *freefare_get_new_tags (nfc_device_t *device, MifareTag *known_tags) );
  - remove vanished tags (e.g. MifareTag *freefare_remove_vanished_tags (nfc_device_t *device, MifareTag *known_tags) ).

An additional set of functions for list manipulation (boolean set manipulation 
(e.g. union, difference)) would be a plus.

The idea is that we can either have a single list of tags that is updated, or 
"many" lists that we can combine the way we want.  My personal preference is 
for the first (smallest) API change, but I am not 100% sure it fits your 
needs.  AFAICS, you should just have to backup the (old) list of pointer to 
MifareTags and them compare these pointers with the one for the new MifareTags 
(since tags would not be reallocated, their address would not 
change, and copying the pointer to a tag does not ruin the stateful model).

Original comment by romain.t...@gmail.com on 26 Apr 2010 at 8:42

GoogleCodeExporter commented 9 years ago
After talking with Romuald over IM, we decided to drop the 
freefare_duplicate_tag() function and complete the 
documentation about how to use freefare_free_tags() and freefare_free_tag().

Original comment by romain.t...@gmail.com on 26 Apr 2010 at 10:31

GoogleCodeExporter commented 9 years ago
This issue was updated by revision r268.

Remove the freefare_duplicate_tag() function.

Original comment by romain.t...@gmail.com on 26 Apr 2010 at 10:34

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r269.

Original comment by romain.t...@gmail.com on 26 Apr 2010 at 10:53