[PATCH] USB speedtouch: per vcc data cleanups

Use struct list_head rather than a singly linked list in udsl_vcc_data.  Reject
attempts to open multiple vccs with the same vpi/vci pair.  Some cleanups too.

drivers/usb/misc/speedtch.c

@@ -150,10 +150,9 @@ struct udsl_control {
 
 struct udsl_vcc_data {
 	/* vpi/vci lookup */
-	struct udsl_vcc_data *next;
-	unsigned int vpi;
-	unsigned int vci;
-	unsigned long atmHeader;
+	struct list_head list;
+	short vpi;
+	int vci;
 	struct atm_vcc *vcc;
 
 	/* raw cell reassembly */
@@ -175,7 +174,7 @@ struct udsl_instance_data {
 
 	/* atm device part */
 	struct atm_dev *atm_dev;
-	struct udsl_vcc_data *vcc_list;
+	struct list_head vcc_list;
 
 	/* receiving */
 	struct udsl_receiver all_receivers [UDSL_NUMBER_RCV_URBS];
@@ -247,11 +246,19 @@ static struct usb_driver udsl_usb_driver = {
 **  decode  **
 *************/
 
-#define ATM_HDR_VPVC_MASK		(ATM_HDR_VPI_MASK | ATM_HDR_VCI_MASK)
+static inline struct udsl_vcc_data *udsl_find_vcc (struct udsl_instance_data *instance, short vpi, int vci)
+{
+	struct udsl_vcc_data *vcc;
+
+	list_for_each_entry (vcc, &instance->vcc_list, list)
+		if ((vcc->vpi == vpi) && (vcc->vci == vci))
+			return vcc;
+	return NULL;
+}
 
-static struct sk_buff *udsl_decode_rawcell (struct udsl_vcc_data *list, struct sk_buff *skb, struct udsl_vcc_data **ctx)
+static struct sk_buff *udsl_decode_rawcell (struct udsl_instance_data *instance, struct sk_buff *skb, struct udsl_vcc_data **ctx)
 {
-	if (!list || !skb || !ctx)
+	if (!instance || !skb || !ctx)
 		return NULL;
 	if (!skb->data || !skb->tail)
 		return NULL;
@@ -259,68 +266,66 @@ static struct sk_buff *udsl_decode_rawcell (struct udsl_vcc_data *list, struct s
 	while (skb->len) {
 		unsigned char *cell = skb->data;
 		unsigned char *cell_payload;
-		struct udsl_vcc_data *vcc = list;
-		unsigned long atmHeader =
-		    ((unsigned long) (cell[0]) << 24) | ((unsigned long) (cell[1]) << 16) |
-		    ((unsigned long) (cell[2]) << 8) | (cell[3] & 0xff);
+		struct udsl_vcc_data *vcc;
+		short vpi;
+		int vci;
 
-		dbg ("udsl_decode_rawcell (0x%p, 0x%p, 0x%p) called", list, skb, ctx);
+		vpi = ((cell[0] & 0x0f) << 4) | (cell[1] >> 4);
+		vci = ((cell[1] & 0x0f) << 12) | (cell[2] << 4) | (cell[3] >> 4);
+
+		dbg ("udsl_decode_rawcell (0x%p, 0x%p, 0x%p) called", instance, skb, ctx);
 		dbg ("udsl_decode_rawcell skb->data %p, skb->tail %p", skb->data, skb->tail);
 
 		/* here should the header CRC check be... */
 
-		/* look up correct vcc */
-		for (;
-		     vcc
-		     && ((vcc->atmHeader & ATM_HDR_VPVC_MASK) != (atmHeader & ATM_HDR_VPVC_MASK));
-		     vcc = vcc->next);
-
-		dbg ("udsl_decode_rawcell found vcc %p for packet on vpi %d, vci %d", vcc,
-			(int) ((atmHeader & ATM_HDR_VPI_MASK) >> ATM_HDR_VPI_SHIFT),
-			(int) ((atmHeader & ATM_HDR_VCI_MASK) >> ATM_HDR_VCI_SHIFT));
-
-		if (vcc && (skb->len >= 53)) {
-			cell_payload = cell + 5;
-
-			if (!vcc->reasBuffer)
-				vcc->reasBuffer = dev_alloc_skb (vcc->mtu);
-
-			/* if alloc fails, we just drop the cell. it is possible that we can still
-			 * receive cells on other vcc's
-			 */
-			if (vcc->reasBuffer) {
-				/* if (buffer overrun) discard received cells until now */
-				if ((vcc->reasBuffer->len) > (vcc->mtu - 48))
-					skb_trim (vcc->reasBuffer, 0);
-
-				/* copy data */
-				memcpy (vcc->reasBuffer->tail, cell_payload, 48);
-				skb_put (vcc->reasBuffer, 48);
-
-				/* check for end of buffer */
-				if (cell[3] & 0x2) {
-					struct sk_buff *tmp;
-
-					/* the aal5 buffer ends here, cut the buffer. */
-					/* buffer will always have at least one whole cell, so */
-					/* don't need to check return from skb_pull */
-					skb_pull (skb, 53);
-					*ctx = vcc;
-					tmp = vcc->reasBuffer;
-					vcc->reasBuffer = NULL;
-
-					dbg ("udsl_decode_rawcell returns ATM_AAL5 pdu 0x%p with length %d", tmp, tmp->len);
-					return tmp;
+		if (!(vcc = udsl_find_vcc (instance, vpi, vci)))
+			dbg ("udsl_decode_rawcell: no vcc found for packet on vpi %d, vci %d", vpi, vci);
+		else {
+			dbg ("udsl_decode_rawcell found vcc %p for packet on vpi %d, vci %d", vcc, vpi, vci);
+
+			if (skb->len >= 53) {
+				cell_payload = cell + 5;
+
+				if (!vcc->reasBuffer)
+					vcc->reasBuffer = dev_alloc_skb (vcc->mtu);
+
+				/* if alloc fails, we just drop the cell. it is possible that we can still
+				 * receive cells on other vcc's
+				 */
+				if (vcc->reasBuffer) {
+					/* if (buffer overrun) discard received cells until now */
+					if ((vcc->reasBuffer->len) > (vcc->mtu - 48))
+						skb_trim (vcc->reasBuffer, 0);
+
+					/* copy data */
+					memcpy (vcc->reasBuffer->tail, cell_payload, 48);
+					skb_put (vcc->reasBuffer, 48);
+
+					/* check for end of buffer */
+					if (cell[3] & 0x2) {
+						struct sk_buff *tmp;
+
+						/* the aal5 buffer ends here, cut the buffer. */
+						/* buffer will always have at least one whole cell, so */
+						/* don't need to check return from skb_pull */
+						skb_pull (skb, 53);
+						*ctx = vcc;
+						tmp = vcc->reasBuffer;
+						vcc->reasBuffer = NULL;
+
+						dbg ("udsl_decode_rawcell returns ATM_AAL5 pdu 0x%p with length %d", tmp, tmp->len);
+						return tmp;
+					}
 				}
+				/* flush the cell */
+				/* buffer will always contain at least one whole cell, so don't */
+				/* need to check return value from skb_pull */
+				skb_pull (skb, 53);
+			} else {
+				/* If data is corrupt and skb doesn't hold a whole cell, flush the lot */
+				if (skb_pull (skb, 53) == NULL)
+					return NULL;
 			}
-			/* flush the cell */
-			/* buffer will always contain at least one whole cell, so don't */
-			/* need to check return value from skb_pull */
-			skb_pull (skb, 53);
-		} else {
-			/* If data is corrupt and skb doesn't hold a whole cell, flush the lot */
-			if (skb_pull (skb, 53) == NULL)
-				return NULL;
 		}
 	}
 
@@ -342,8 +347,7 @@ static struct sk_buff *udsl_decode_aal5 (struct udsl_vcc_data *ctx, struct sk_bu
 	    (skb->tail[-4] << 24) + (skb->tail[-3] << 16) + (skb->tail[-2] << 8) + skb->tail[-1];
 	pdu_length = ((length + 47 + 8) / 48) * 48;
 
-	dbg ("udsl_decode_aal5: skb->len = %d, length = %d, pdu_crc = 0x%x, pdu_length = %d",
-		skb->len, length, pdu_crc, pdu_length);
+	dbg ("udsl_decode_aal5: skb->len = %d, length = %d, pdu_crc = 0x%x, pdu_length = %d", skb->len, length, pdu_crc, pdu_length);
 
 	/* is skb long enough ? */
 	if (skb->len < pdu_length) {
@@ -354,8 +358,7 @@ static struct sk_buff *udsl_decode_aal5 (struct udsl_vcc_data *ctx, struct sk_bu
 
 	/* is skb too long ? */
 	if (skb->len > pdu_length) {
-		dbg ("udsl_decode_aal5: Warning: readjusting illegal size %d -> %d",
-			skb->len, pdu_length);
+		dbg ("udsl_decode_aal5: Warning: readjusting illegal size %d -> %d", skb->len, pdu_length);
 		/* buffer is too long. we can try to recover
 		 * if we discard the first part of the skb.
 		 * the crc will decide whether this was ok
@@ -548,7 +551,7 @@ static void udsl_process_receive (unsigned long data)
 			dbg ("skb->len = %d", skb->len);
 			PACKETDEBUG (skb->data, skb->len);
 
-			while ((new = udsl_decode_rawcell (instance->vcc_list, skb, &atmsar_vcc))) {
+			while ((new = udsl_decode_rawcell (instance, skb, &atmsar_vcc))) {
 				dbg ("(after cell processing)skb->len = %d", new->len);
 
 				tmp = new;
@@ -923,94 +926,81 @@ static int udsl_atm_open (struct atm_vcc *vcc, short vpi, int vci)
 		return -ENODEV;
 	}
 
+	if ((vpi == ATM_VPI_ANY) || (vci == ATM_VCI_ANY))
+		return -EINVAL;
+
 	/* only support AAL5 */
 	if (vcc->qos.aal != ATM_AAL5)
 		return -EINVAL;
 
+	if (udsl_find_vcc (instance, vpi, vci))
+		return -EADDRINUSE;
+
 	if (!(new = kmalloc (sizeof (struct udsl_vcc_data), GFP_KERNEL)))
 		return -ENOMEM;
 
-	MOD_INC_USE_COUNT;
-
 	memset (new, 0, sizeof (struct udsl_vcc_data));
 	new->vcc = vcc;
 	new->vpi = vpi;
 	new->vci = vci;
-
 	new->mtu = UDSL_MAX_AAL5_MRU;
 
-	new->atmHeader = ((unsigned long) vpi << ATM_HDR_VPI_SHIFT) |
-			 ((unsigned long) vci << ATM_HDR_VCI_SHIFT);
-	new->reasBuffer = NULL;
-
-	new->next = instance->vcc_list;
-	instance->vcc_list = new;
-
-	dbg ("Allocated new SARLib vcc 0x%p with vpi %d vci %d", new, vpi, vci);
-
 	vcc->dev_data = new;
-
 	vcc->vpi = vpi;
 	vcc->vci = vci;
+
+	list_add (&new->list, &instance->vcc_list);
+
 	set_bit (ATM_VF_ADDR, &vcc->flags);
 	set_bit (ATM_VF_PARTIAL, &vcc->flags);
 	set_bit (ATM_VF_READY, &vcc->flags);
 
+	dbg ("Allocated new SARLib vcc 0x%p with vpi %d vci %d", new, vpi, vci);
+
+	MOD_INC_USE_COUNT;
+
 	if (instance->firmware_loaded)
 		udsl_fire_receivers (instance);
 
 	dbg ("udsl_atm_open successful");
+
 	return 0;
 }
 
 static void udsl_atm_close (struct atm_vcc *vcc)
 {
 	struct udsl_instance_data *instance = vcc->dev->dev_data;
-	struct udsl_vcc_data *work;
+	struct udsl_vcc_data *vcc_data = vcc->dev_data;
 
 	dbg ("udsl_atm_close called");
 
-	if (!instance) {
-		dbg ("NULL instance!");
+	if (!instance || !vcc_data) {
+		dbg ("NULL data!");
 		return;
 	}
 
-	/* freeing resources */
-	/* cancel all sends on this vcc */
-	udsl_cancel_send (instance, vcc);
+	dbg ("Deallocating SARLib vcc 0x%p with vpi %d vci %d", vcc_data, vcc_data->vpi, vcc_data->vci);
 
-	if (instance->vcc_list == vcc->dev_data) {
-		instance->vcc_list = instance->vcc_list->next;
-	} else {
-		for (work = instance->vcc_list; work && work->next && (work->next != vcc->dev_data); work = work->next);
-
-		/* return if not found */
-		if (work->next != vcc->dev_data)
-			BUG ();
-
-		work->next = work->next->next;
-	}
-
-	if (((struct udsl_vcc_data *)vcc->dev_data)->reasBuffer) {
-		dev_kfree_skb (((struct udsl_vcc_data *)vcc->dev_data)->reasBuffer);
-	}
+	udsl_cancel_send (instance, vcc);
 
-	dbg ("Deallocated SARLib vcc 0x%p with vpi %d vci %d", vcc->dev_data, vcc->dev_data->vpi, vcc->dev_data->vci);
+	list_del (&vcc_data->list);
 
-	kfree (vcc->dev_data);
+	if (vcc_data->reasBuffer)
+		kfree_skb (vcc_data->reasBuffer);
+	vcc_data->reasBuffer = NULL;
 
+	kfree (vcc_data);
 	vcc->dev_data = NULL;
-	clear_bit (ATM_VF_PARTIAL, &vcc->flags);
 
-	/* freeing address */
 	vcc->vpi = ATM_VPI_UNSPEC;
 	vcc->vci = ATM_VCI_UNSPEC;
+	clear_bit (ATM_VF_READY, &vcc->flags);
+	clear_bit (ATM_VF_PARTIAL, &vcc->flags);
 	clear_bit (ATM_VF_ADDR, &vcc->flags);
 
 	MOD_DEC_USE_COUNT;
 
 	dbg ("udsl_atm_close successful");
-	return;
 }
 
 static int udsl_atm_ioctl (struct atm_dev *dev, unsigned int cmd, void *arg)
@@ -1089,6 +1079,8 @@ static int udsl_usb_probe (struct usb_interface *intf, const struct usb_device_i
 
 	instance->usb_dev = dev;
 
+	INIT_LIST_HEAD (&instance->vcc_list);
+
 	spin_lock_init (&instance->spare_receivers_lock);
 	INIT_LIST_HEAD (&instance->spare_receivers);