* [RFC] irda: irttp: allow zero byte packets
@ 2010-11-09 23:19 Wolfram Sang
2010-11-16 17:51 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Wolfram Sang @ 2010-11-09 23:19 UTC (permalink / raw)
To: irda-users; +Cc: netdev, Wolfram Sang, Samuel Ortiz
Sending zero byte packets is not neccessarily an error (AF_INET accepts it,
too), so just apply a shortcut. This was discovered because of a non-working
software with WINE. See
http://bugs.winehq.org/show_bug.cgi?id=19397#c86
http://thread.gmane.org/gmane.linux.irda.general/1643
for very detailed debugging information and a testcase. Kudos to Wolfgang for
those!
Reported-by: Wolfgang Schwotzer <wolfgang.schwotzer@gmx.net>
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Tested-by: Mike Evans <mike.evans@cardolan.com>
Cc: Samuel Ortiz <samuel@sortiz.org>
---
I found Wolfgang's very detailed report while looking for WINE-bugreports
affecting the kernel somehow. His mail sadly went to an almost dead
mailing-list and he told me he lost interest meanwhile. This is why I picked
the issue up and created this straightforward patch which helps the case at
least (thanks Mike for testing!).
net/irda/irttp.c | 25 +++++++++++++++++++------
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/net/irda/irttp.c b/net/irda/irttp.c
index 285761e..6cfaeaf 100644
--- a/net/irda/irttp.c
+++ b/net/irda/irttp.c
@@ -550,16 +550,23 @@ EXPORT_SYMBOL(irttp_close_tsap);
*/
int irttp_udata_request(struct tsap_cb *self, struct sk_buff *skb)
{
+ int ret = -1;
+
IRDA_ASSERT(self != NULL, return -1;);
IRDA_ASSERT(self->magic == TTP_TSAP_MAGIC, return -1;);
IRDA_ASSERT(skb != NULL, return -1;);
IRDA_DEBUG(4, "%s()\n", __func__);
+ /* Take shortcut on zero byte packets */
+ if (skb->len == 0) {
+ ret = 0;
+ goto err;
+ }
+
/* Check that nothing bad happens */
- if ((skb->len == 0) || (!self->connected)) {
- IRDA_DEBUG(1, "%s(), No data, or not connected\n",
- __func__);
+ if (!self->connected) {
+ IRDA_DEBUG(1, "%s(), Not connected\n", __func__);
goto err;
}
@@ -576,7 +583,7 @@ int irttp_udata_request(struct tsap_cb *self, struct sk_buff *skb)
err:
dev_kfree_skb(skb);
- return -1;
+ return ret;
}
EXPORT_SYMBOL(irttp_udata_request);
@@ -599,9 +606,15 @@ int irttp_data_request(struct tsap_cb *self, struct sk_buff *skb)
IRDA_DEBUG(2, "%s() : queue len = %d\n", __func__,
skb_queue_len(&self->tx_queue));
+ /* Take shortcut on zero byte packets */
+ if (skb->len == 0) {
+ ret = 0;
+ goto err;
+ }
+
/* Check that nothing bad happens */
- if ((skb->len == 0) || (!self->connected)) {
- IRDA_WARNING("%s: No data, or not connected\n", __func__);
+ if (!self->connected) {
+ IRDA_WARNING("%s: Not connected\n", __func__);
ret = -ENOTCONN;
goto err;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC] irda: irttp: allow zero byte packets
2010-11-09 23:19 [RFC] irda: irttp: allow zero byte packets Wolfram Sang
@ 2010-11-16 17:51 ` David Miller
2010-11-16 19:40 ` [PATCH] net: irda: irttp: sync error paths of data- and udata-requests Wolfram Sang
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2010-11-16 17:51 UTC (permalink / raw)
To: w.sang; +Cc: irda-users, netdev, samuel
From: Wolfram Sang <w.sang@pengutronix.de>
Date: Wed, 10 Nov 2010 00:19:47 +0100
> Sending zero byte packets is not neccessarily an error (AF_INET accepts it,
> too), so just apply a shortcut. This was discovered because of a non-working
> software with WINE. See
>
> http://bugs.winehq.org/show_bug.cgi?id=19397#c86
> http://thread.gmane.org/gmane.linux.irda.general/1643
>
> for very detailed debugging information and a testcase. Kudos to Wolfgang for
> those!
>
> Reported-by: Wolfgang Schwotzer <wolfgang.schwotzer@gmx.net>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Tested-by: Mike Evans <mike.evans@cardolan.com>
This looks good to me, applied, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] net: irda: irttp: sync error paths of data- and udata-requests
2010-11-16 17:51 ` David Miller
@ 2010-11-16 19:40 ` Wolfram Sang
2010-11-18 20:24 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Wolfram Sang @ 2010-11-16 19:40 UTC (permalink / raw)
To: netdev; +Cc: Wolfram Sang, Samuel Ortiz, David Miller
irttp_data_request() returns meaningful errorcodes, while irttp_udata_request()
just returns -1 in similar situations. Sync the two and the loglevels of the
accompanying output.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: David Miller <davem@davemloft.net>
---
Thank you David for picking up the zero-byte-packet-patch. Now as it was
applied, this one might be interesting, too (on top of it)? Nothing seriously
needed, but looks more proper IMHO. LXR says that are callers of these
functions check with < 0 anyhow.
net/irda/irttp.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/irda/irttp.c b/net/irda/irttp.c
index 6cfaeaf..f6054f9 100644
--- a/net/irda/irttp.c
+++ b/net/irda/irttp.c
@@ -550,7 +550,7 @@ EXPORT_SYMBOL(irttp_close_tsap);
*/
int irttp_udata_request(struct tsap_cb *self, struct sk_buff *skb)
{
- int ret = -1;
+ int ret;
IRDA_ASSERT(self != NULL, return -1;);
IRDA_ASSERT(self->magic == TTP_TSAP_MAGIC, return -1;);
@@ -566,13 +566,14 @@ int irttp_udata_request(struct tsap_cb *self, struct sk_buff *skb)
/* Check that nothing bad happens */
if (!self->connected) {
- IRDA_DEBUG(1, "%s(), Not connected\n", __func__);
+ IRDA_WARNING("%s(), Not connected\n", __func__);
+ ret = -ENOTCONN;
goto err;
}
if (skb->len > self->max_seg_size) {
- IRDA_DEBUG(1, "%s(), UData is too large for IrLAP!\n",
- __func__);
+ IRDA_ERROR("%s(), UData is too large for IrLAP!\n", __func__);
+ ret = -EMSGSIZE;
goto err;
}
--
1.7.2.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: irda: irttp: sync error paths of data- and udata-requests
2010-11-16 19:40 ` [PATCH] net: irda: irttp: sync error paths of data- and udata-requests Wolfram Sang
@ 2010-11-18 20:24 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2010-11-18 20:24 UTC (permalink / raw)
To: w.sang; +Cc: netdev, sameo
From: Wolfram Sang <w.sang@pengutronix.de>
Date: Tue, 16 Nov 2010 20:40:02 +0100
> irttp_data_request() returns meaningful errorcodes, while irttp_udata_request()
> just returns -1 in similar situations. Sync the two and the loglevels of the
> accompanying output.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: David Miller <davem@davemloft.net>
> ---
>
> Thank you David for picking up the zero-byte-packet-patch. Now as it was
> applied, this one might be interesting, too (on top of it)? Nothing seriously
> needed, but looks more proper IMHO. LXR says that are callers of these
> functions check with < 0 anyhow.
Better error code reporting is always appreciated :-)
Applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-18 20:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-09 23:19 [RFC] irda: irttp: allow zero byte packets Wolfram Sang
2010-11-16 17:51 ` David Miller
2010-11-16 19:40 ` [PATCH] net: irda: irttp: sync error paths of data- and udata-requests Wolfram Sang
2010-11-18 20:24 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.