* [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
@ 2015-04-10 14:48 ` Giedrius Statkevičius
0 siblings, 0 replies; 10+ messages in thread
From: Giedrius Statkevičius @ 2015-04-10 14:48 UTC (permalink / raw)
To: lidza.louina, markh
Cc: gregkh, driverdev-devel, devel, linux-kernel, Giedrius Statkevičius
Remove the dead code protected by in_user in dgnc_tty_write() because it is set
to 0 and never changed to 1 thus the code in ifs never gets executed.
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
---
v2: Just remove the dead code protected by in_user and join the first and third
patches.
drivers/staging/dgnc/dgnc_tty.c | 45 +----------------------------------------
1 file changed, 1 insertion(+), 44 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index ce4187f..cf2eb21 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -42,16 +42,11 @@
#include "dgnc_sysfs.h"
#include "dgnc_utils.h"
-#define init_MUTEX(sem) sema_init(sem, 1)
-#define DECLARE_MUTEX(name) \
- struct semaphore name = __SEMAPHORE_INITIALIZER(name, 1)
-
/*
* internal variables
*/
static struct dgnc_board *dgnc_BoardsByMajor[256];
static unsigned char *dgnc_TmpWriteBuf;
-static DECLARE_MUTEX(dgnc_TmpWriteSem);
/*
* Default transparent print information.
@@ -1705,7 +1700,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
ushort tail;
ushort tmask;
uint remain;
- int from_user = 0;
if (tty == NULL || dgnc_TmpWriteBuf == NULL)
return 0;
@@ -1785,38 +1779,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
if (count <= 0)
goto exit_retry;
- if (from_user) {
-
- count = min(count, WRITEBUFLEN);
-
- spin_unlock_irqrestore(&ch->ch_lock, flags);
-
- /*
- * If data is coming from user space, copy it into a temporary
- * buffer so we don't get swapped out while doing the copy to
- * the board.
- */
- /* we're allowed to block if it's from_user */
- if (down_interruptible(&dgnc_TmpWriteSem))
- return -EINTR;
-
- /*
- * copy_from_user() returns the number
- * of bytes that could *NOT* be copied.
- */
- count -= copy_from_user(dgnc_TmpWriteBuf, (const unsigned char __user *) buf, count);
-
- if (!count) {
- up(&dgnc_TmpWriteSem);
- return -EFAULT;
- }
-
- spin_lock_irqsave(&ch->ch_lock, flags);
-
- buf = dgnc_TmpWriteBuf;
-
- }
-
n = count;
/*
@@ -1853,12 +1815,7 @@ static int dgnc_tty_write(struct tty_struct *tty,
ch->ch_cpstime += (HZ * count) / ch->ch_digi.digi_maxcps;
}
- if (from_user) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- up(&dgnc_TmpWriteSem);
- } else {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- }
+ spin_unlock_irqrestore(&ch->ch_lock, flags);
if (count) {
/*
--
2.3.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
@ 2015-04-10 14:48 ` Giedrius Statkevičius
0 siblings, 0 replies; 10+ messages in thread
From: Giedrius Statkevičius @ 2015-04-10 14:48 UTC (permalink / raw)
To: lidza.louina, markh
Cc: gregkh, driverdev-devel, devel, linux-kernel, Giedrius Statkevičius
Remove the dead code protected by in_user in dgnc_tty_write() because it
is set to 0 and never changed to 1 thus the code in ifs never gets
executed.
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
---
v2: Just remove the dead code protected by in_user and join the first and third
patches.
drivers/staging/dgnc/dgnc_tty.c | 45 +----------------------------------------
1 file changed, 1 insertion(+), 44 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index ce4187f..cf2eb21 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -42,16 +42,11 @@
#include "dgnc_sysfs.h"
#include "dgnc_utils.h"
-#define init_MUTEX(sem) sema_init(sem, 1)
-#define DECLARE_MUTEX(name) \
- struct semaphore name = __SEMAPHORE_INITIALIZER(name, 1)
-
/*
* internal variables
*/
static struct dgnc_board *dgnc_BoardsByMajor[256];
static unsigned char *dgnc_TmpWriteBuf;
-static DECLARE_MUTEX(dgnc_TmpWriteSem);
/*
* Default transparent print information.
@@ -1705,7 +1700,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
ushort tail;
ushort tmask;
uint remain;
- int from_user = 0;
if (tty == NULL || dgnc_TmpWriteBuf == NULL)
return 0;
@@ -1785,38 +1779,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
if (count <= 0)
goto exit_retry;
- if (from_user) {
-
- count = min(count, WRITEBUFLEN);
-
- spin_unlock_irqrestore(&ch->ch_lock, flags);
-
- /*
- * If data is coming from user space, copy it into a temporary
- * buffer so we don't get swapped out while doing the copy to
- * the board.
- */
- /* we're allowed to block if it's from_user */
- if (down_interruptible(&dgnc_TmpWriteSem))
- return -EINTR;
-
- /*
- * copy_from_user() returns the number
- * of bytes that could *NOT* be copied.
- */
- count -= copy_from_user(dgnc_TmpWriteBuf, (const unsigned char __user *) buf, count);
-
- if (!count) {
- up(&dgnc_TmpWriteSem);
- return -EFAULT;
- }
-
- spin_lock_irqsave(&ch->ch_lock, flags);
-
- buf = dgnc_TmpWriteBuf;
-
- }
-
n = count;
/*
@@ -1853,12 +1815,7 @@ static int dgnc_tty_write(struct tty_struct *tty,
ch->ch_cpstime += (HZ * count) / ch->ch_digi.digi_maxcps;
}
- if (from_user) {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- up(&dgnc_TmpWriteSem);
- } else {
- spin_unlock_irqrestore(&ch->ch_lock, flags);
- }
+ spin_unlock_irqrestore(&ch->ch_lock, flags);
if (count) {
/*
--
2.3.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2 RESEND] staging: dgnc: remove redundant check
2015-04-10 14:48 ` Giedrius Statkevičius
@ 2015-04-10 14:48 ` Giedrius Statkevičius
-1 siblings, 0 replies; 10+ messages in thread
From: Giedrius Statkevičius @ 2015-04-10 14:48 UTC (permalink / raw)
To: lidza.louina, markh
Cc: gregkh, driverdev-devel, devel, linux-kernel, Giedrius Statkevičius
count doesn't get changed in between identical checks in dgnc_tty_write() so
remove the second check
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
---
v2: no change
drivers/staging/dgnc/dgnc_tty.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index cf2eb21..61d5a8e 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1773,12 +1773,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
ch->ch_flags &= ~CH_PRON;
}
- /*
- * If there is nothing left to copy, or I can't handle any more data, leave.
- */
- if (count <= 0)
- goto exit_retry;
-
n = count;
/*
--
2.3.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2 RESEND] staging: dgnc: remove redundant check
@ 2015-04-10 14:48 ` Giedrius Statkevičius
0 siblings, 0 replies; 10+ messages in thread
From: Giedrius Statkevičius @ 2015-04-10 14:48 UTC (permalink / raw)
To: lidza.louina, markh
Cc: gregkh, driverdev-devel, devel, linux-kernel, Giedrius Statkevičius
count doesn't get changed in between identical checks in
dgnc_tty_write() so remove the second check
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
---
v2: no change
drivers/staging/dgnc/dgnc_tty.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index cf2eb21..61d5a8e 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1773,12 +1773,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
ch->ch_flags &= ~CH_PRON;
}
- /*
- * If there is nothing left to copy, or I can't handle any more data, leave.
- */
- if (count <= 0)
- goto exit_retry;
-
n = count;
/*
--
2.3.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
2015-04-10 14:48 ` Giedrius Statkevičius
(?)
(?)
@ 2015-04-11 4:51 ` Sudip Mukherjee
2015-04-11 12:20 ` Giedrius Statkevičius
-1 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2015-04-11 4:51 UTC (permalink / raw)
To: Giedrius Statkevičius
Cc: lidza.louina, markh, gregkh, driverdev-devel, devel, linux-kernel
On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> Remove the dead code protected by in_user in dgnc_tty_write() because it is set
> to 0 and never changed to 1 thus the code in ifs never gets executed.
dgnc_tty_write() is being called by dgnc_tty_put_char() and it is also
the write callback function of struct tty_operations, so I think the
correct fix will be to use from_user and make it 0 when
dgnc_tty_put_char() calls this function else make it 1 to inform the
function that the data wil be coming from the userspace. Maybe some
thing like this:
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index ce4187f..96eaf5b 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1682,10 +1682,15 @@ static int dgnc_tty_put_char(struct tty_struct *tty, unsigned char c)
/*
* Simply call tty_write.
*/
- dgnc_tty_write(tty, &c, 1);
+ __dgnc_tty_write(tty, &c, 1, 0);
return 1;
}
+static int dgnc_tty_write(struct tty_struct *tty,
+ const unsigned char *buf, int count)
+{
+ __dgnc_tty_write(tty, buf, count, 1);
+}
/*
* dgnc_tty_write()
@@ -1693,8 +1698,8 @@ static int dgnc_tty_put_char(struct tty_struct *tty, unsigned char c)
* Take data from the user or kernel and send it out to the FEP.
* In here exists all the Transparent Print magic as well.
*/
-static int dgnc_tty_write(struct tty_struct *tty,
- const unsigned char *buf, int count)
+static int __dgnc_tty_write(struct tty_struct *tty,
+ const unsigned char *buf, int count, int from_user)
{
struct channel_t *ch = NULL;
struct un_t *un = NULL;
@@ -1705,7 +1710,6 @@ static int dgnc_tty_write(struct tty_struct *tty,
ushort tail;
ushort tmask;
uint remain;
- int from_user = 0;
if (tty == NULL || dgnc_TmpWriteBuf == NULL)
return 0;
regards
sudip
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
2015-04-11 4:51 ` [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write() Sudip Mukherjee
@ 2015-04-11 12:20 ` Giedrius Statkevičius
0 siblings, 0 replies; 10+ messages in thread
From: Giedrius Statkevičius @ 2015-04-11 12:20 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Giedrius Statkevičius, lidza.louina, markh, gregkh,
driverdev-devel, devel, linux-kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1383 bytes --]
On Sat, 11 Apr 2015, Sudip Mukherjee wrote:
> On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> > Remove the dead code protected by in_user in dgnc_tty_write() because it is set
> > to 0 and never changed to 1 thus the code in ifs never gets executed.
> dgnc_tty_write() is being called by dgnc_tty_put_char() and it is also
> the write callback function of struct tty_operations, so I think the
> correct fix will be to use from_user and make it 0 when
> dgnc_tty_put_char() calls this function else make it 1 to inform the
> function that the data wil be coming from the userspace. Maybe some
> thing like this:
Well, I think this is wrong because:
* parameter of write member of struct tty_operations buf is not tagged with
"__user" so it should be safe to just memcpy() from it
* Looked through some other write operations in other tty drivers and I've never
seen copy_from_user() used on buf argument of write operation - always
memcpy() or some other similar function
* Ldd3 (and the comments in tty_driver.h) says that write could be called from
interrupt context too so it can't sleep and thus use copy_from_user()
While looking through I've caught that dgnc_TmpWriteBuf could be also deleted
because it will be unused if this patch goes through. If it does then I'll send
another one (or should I send a v3?)
Su pagarba / Regards,
Giedrius
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
@ 2015-04-11 12:20 ` Giedrius Statkevičius
0 siblings, 0 replies; 10+ messages in thread
From: Giedrius Statkevičius @ 2015-04-11 12:20 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: devel, lidza.louina, driverdev-devel, linux-kernel, gregkh
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1383 bytes --]
On Sat, 11 Apr 2015, Sudip Mukherjee wrote:
> On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> > Remove the dead code protected by in_user in dgnc_tty_write() because it is set
> > to 0 and never changed to 1 thus the code in ifs never gets executed.
> dgnc_tty_write() is being called by dgnc_tty_put_char() and it is also
> the write callback function of struct tty_operations, so I think the
> correct fix will be to use from_user and make it 0 when
> dgnc_tty_put_char() calls this function else make it 1 to inform the
> function that the data wil be coming from the userspace. Maybe some
> thing like this:
Well, I think this is wrong because:
* parameter of write member of struct tty_operations buf is not tagged with
"__user" so it should be safe to just memcpy() from it
* Looked through some other write operations in other tty drivers and I've never
seen copy_from_user() used on buf argument of write operation - always
memcpy() or some other similar function
* Ldd3 (and the comments in tty_driver.h) says that write could be called from
interrupt context too so it can't sleep and thus use copy_from_user()
While looking through I've caught that dgnc_TmpWriteBuf could be also deleted
because it will be unused if this patch goes through. If it does then I'll send
another one (or should I send a v3?)
Su pagarba / Regards,
Giedrius
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
2015-04-11 12:20 ` Giedrius Statkevičius
(?)
@ 2015-04-11 12:59 ` Sudip Mukherjee
-1 siblings, 0 replies; 10+ messages in thread
From: Sudip Mukherjee @ 2015-04-11 12:59 UTC (permalink / raw)
To: Giedrius Statkevičius
Cc: lidza.louina, markh, gregkh, driverdev-devel, devel, linux-kernel
On Sat, Apr 11, 2015 at 03:20:43PM +0300, Giedrius Statkevičius wrote:
> On Sat, 11 Apr 2015, Sudip Mukherjee wrote:
>
> > On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> Well, I think this is wrong because:
yes. I looked at many of the tty drivers and all of them have used
memcpy(). Sorry for my last mail, i should have written that after
seeing all other similar drivers and maybe once in a while i should
go back to LDD3. :)
regards
sudip
>
> Su pagarba / Regards,
> Giedrius
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
2015-04-10 14:48 ` Giedrius Statkevičius
@ 2015-05-03 18:49 ` Greg KH
-1 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2015-05-03 18:49 UTC (permalink / raw)
To: Giedrius Statkevičius
Cc: lidza.louina, markh, driverdev-devel, devel, linux-kernel
On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> Remove the dead code protected by in_user in dgnc_tty_write() because it is set
> to 0 and never changed to 1 thus the code in ifs never gets executed.
Please wrap your commit lines at 72 columns in the future.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write()
@ 2015-05-03 18:49 ` Greg KH
0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2015-05-03 18:49 UTC (permalink / raw)
To: Giedrius Statkevičius
Cc: lidza.louina, driverdev-devel, linux-kernel, devel
On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> Remove the dead code protected by in_user in dgnc_tty_write() because it is set
> to 0 and never changed to 1 thus the code in ifs never gets executed.
Please wrap your commit lines at 72 columns in the future.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-03 19:04 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-10 14:48 [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write() Giedrius Statkevičius
2015-04-10 14:48 ` Giedrius Statkevičius
2015-04-10 14:48 ` [PATCH v2 2/2 RESEND] staging: dgnc: remove redundant check Giedrius Statkevičius
2015-04-10 14:48 ` Giedrius Statkevičius
2015-04-11 4:51 ` [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty_write() Sudip Mukherjee
2015-04-11 12:20 ` Giedrius Statkevičius
2015-04-11 12:20 ` Giedrius Statkevičius
2015-04-11 12:59 ` Sudip Mukherjee
2015-05-03 18:49 ` Greg KH
2015-05-03 18:49 ` Greg KH
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.