[PATCH] ip2: fix compile warnings

[PATCH] ip2: fix compile warnings

[james4765]

This fixes the following compile errors in the ip2 and ip2main drivers:

  CC      drivers/char/ip2main.o
drivers/char/ip2main.c:470: warning: initialization from incompatible pointer type
drivers/char/ip2main.c: In function `ip2_tiocmget':
drivers/char/ip2main.c:2004: warning: unused variable `wait'
drivers/char/ip2/i2lib.c: At top level:
drivers/char/ip2/i2cmd.c:142: warning: `ct89' defined but not used
drivers/char/ip2main.c:205: warning: `set_modem_info' declared `static' but never defined
drivers/char/ip2/i2ellis.c:108: warning: `iiEllisCleanup' defined but not used


Diffstat output:
 ip2/i2cmd.c   |    4 +++-
 ip2/i2cmd.h   |    2 ++
 ip2/i2ellis.c |    2 ++
 ip2main.c     |    6 +++---
 4 files changed, 10 insertions(+), 4 deletions(-)

Signed-off-by: James Nelson <james4765@gmail.com>

diff -urN --exclude='*~' linux-2.6.10-rc3-mm1-original/drivers/char/ip2/i2cmd.c linux-2.6.10-rc3-mm1/drivers/char/ip2/i2cmd.c
--- linux-2.6.10-rc3-mm1-original/drivers/char/ip2/i2cmd.c	2004-12-03 16:53:20.000000000 -0500
+++ linux-2.6.10-rc3-mm1/drivers/char/ip2/i2cmd.c	2004-12-17 16:21:34.592613441 -0500
@@ -139,7 +139,9 @@
 //static UCHAR ct86[]={ 2, BTH,     0x56,0                   }; // RCV_ENABLE
 static UCHAR ct87[] = { 1, BYP,     0x57                     }; // HW_TEST
 //static UCHAR ct88[]={ 3, BTH,     0x58,0,0                 }; // RCV_THRESHOLD
-static UCHAR ct89[]={ 1, BYP,     0x59                     }; // DSS_NOW
+#ifdef ENABLE_DSSNOW
+static UCHAR ct89[]={ 1, BYP,     0x59                     }; // DSS_NOW
+#endif
 //static UCHAR ct90[]={ 3, BYP,     0x5A,0,0                 }; // Set SILO
 //static UCHAR ct91[]={ 2, BYP,     0x5B,0                   }; // timed break
 
diff -urN --exclude='*~' linux-2.6.10-rc3-mm1-original/drivers/char/ip2/i2cmd.h linux-2.6.10-rc3-mm1/drivers/char/ip2/i2cmd.h
--- linux-2.6.10-rc3-mm1-original/drivers/char/ip2/i2cmd.h	2004-12-03 16:52:47.000000000 -0500
+++ linux-2.6.10-rc3-mm1/drivers/char/ip2/i2cmd.h	2004-12-17 16:22:05.205480594 -0500
@@ -637,9 +637,11 @@
 			((cmdSyntaxPtr)(ct88))->cmd[2] = (ms), \
 			(cmdSyntaxPtr)(ct88))
 
+#ifdef ENABLE_DSSNOW
 // Makes the loadware report DSS signals for this channel immediately.
 //
 #define CMD_DSS_NOW (cmdSyntaxPtr)(ct89)
+#endif
 	
 // Set the receive silo parameters 
 // 	timeout is ms idle wait until delivery       (~VTIME)
diff -urN --exclude='*~' linux-2.6.10-rc3-mm1-original/drivers/char/ip2/i2ellis.c linux-2.6.10-rc3-mm1/drivers/char/ip2/i2ellis.c
--- linux-2.6.10-rc3-mm1-original/drivers/char/ip2/i2ellis.c	2004-12-03 16:52:14.000000000 -0500
+++ linux-2.6.10-rc3-mm1/drivers/char/ip2/i2ellis.c	2004-12-17 16:18:33.041123541 -0500
@@ -92,6 +92,7 @@
 	LOCK_INIT(&Dl_spinlock);
 }
 
+#ifdef MODULE
 //******************************************************************************
 // Function:   iiEllisCleanup()
 // Parameters: None
@@ -110,6 +111,7 @@
 		kfree ( pDelayTimer );
 	}
 }
+#endif
 
 //******************************************************************************
 // Function:   iiSetAddress(pB, address, delay)
diff -urN --exclude='*~' linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c linux-2.6.10-rc3-mm1/drivers/char/ip2main.c
--- linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c	2004-12-03 16:55:03.000000000 -0500
+++ linux-2.6.10-rc3-mm1/drivers/char/ip2main.c	2004-12-17 16:24:24.094730049 -0500
@@ -202,7 +202,6 @@
 static void ip2_wait_until_sent(PTTY,int);
 
 static void set_params (i2ChanStrPtr, struct termios *);
-static int set_modem_info(i2ChanStrPtr, unsigned int, unsigned int *);
 static int get_serial_info(i2ChanStrPtr, struct serial_struct __user *);
 static int set_serial_info(i2ChanStrPtr, struct serial_struct __user *);
 
@@ -467,7 +466,7 @@
 static struct tty_operations ip2_ops = {
 	.open            = ip2_open,
 	.close           = ip2_close,
-	.write           = ip2_write,
+	.write           = (void *) ip2_write,
 	.put_char        = ip2_putchar,
 	.flush_chars     = ip2_flush_chars,
 	.write_room      = ip2_write_room,
@@ -2001,7 +2000,6 @@
 static int ip2_tiocmget(struct tty_struct *tty, struct file *file)
 {
 	i2ChanStrPtr pCh = DevTable[tty->index];
-	wait_queue_t wait;
 
 	if (pCh == NULL)
 		return -ENODEV;
@@ -2018,6 +2016,8 @@
 		/\/\|=mhw=|\/\/			*/
 
 #ifdef	ENABLE_DSSNOW
+	wait_queue_t wait;
+
 	i2QueueCommands(PTYPE_BYPASS, pCh, 100, 1, CMD_DSS_NOW);
 
 	init_waitqueue_entry(&wait, current);

Re: [PATCH] ip2: fix compile warnings

[Randy.Dunlap]

james4765@verizon.net wrote:
> This fixes the following compile errors in the ip2 and ip2main drivers:
> 
>   CC      drivers/char/ip2main.o
> drivers/char/ip2main.c:470: warning: initialization from incompatible pointer type


> diff -urN --exclude='*~' linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c linux-2.6.10-rc3-mm1/drivers/char/ip2main.c
> --- linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c	2004-12-03 16:55:03.000000000 -0500
> +++ linux-2.6.10-rc3-mm1/drivers/char/ip2main.c	2004-12-17 16:24:24.094730049 -0500
> @@ -467,7 +466,7 @@
>  static struct tty_operations ip2_ops = {
>  	.open            = ip2_open,
>  	.close           = ip2_close,
> -	.write           = ip2_write,
> +	.write           = (void *) ip2_write,
>  	.put_char        = ip2_putchar,
>  	.flush_chars     = ip2_flush_chars,
>  	.write_room      = ip2_write_room,

The write() prototype in tty_operations is:
	int  (*write)(struct tty_struct * tty,
		      const unsigned char *buf, int count);

Somehow the cast does eliminate the compiler warning (and give
a false sense of correctness).

However, ip2main.c::ip2_write() should be modified like so:

static int
ip2_write( PTTY tty, const unsigned char *pData, int count)

and drop the cast and fix the ip2_write comment (drop old arg 2),
and fix the ip2_write() prototype.
But then you (someone) will have to decide how to handle the
dropped <user> parameter when calling i2Output()...
I don't know the answer to that.
I just changed <user> to 0 to get a clean build of ip2main.o,
but ip2/i2lib.c still needs some work.

-- 
~Randy

Re: [PATCH] ip2: fix compile warnings

[Adrian Bunk]

On Fri, Dec 17, 2004 at 03:47:13PM -0600, james4765@verizon.net wrote:

> This fixes the following compile errors in the ip2 and ip2main drivers:
> 
>   CC      drivers/char/ip2main.o
> drivers/char/ip2main.c:470: warning: initialization from incompatible pointer type
> drivers/char/ip2main.c: In function `ip2_tiocmget':
> drivers/char/ip2main.c:2004: warning: unused variable `wait'
> drivers/char/ip2/i2lib.c: At top level:
> drivers/char/ip2/i2cmd.c:142: warning: `ct89' defined but not used
> drivers/char/ip2main.c:205: warning: `set_modem_info' declared `static' but never defined
> drivers/char/ip2/i2ellis.c:108: warning: `iiEllisCleanup' defined but not used
>...

This are not errors - they are only warnings.
 
> --- linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c	2004-12-03 16:55:03.000000000 -0500
> +++ linux-2.6.10-rc3-mm1/drivers/char/ip2main.c	2004-12-17 16:24:24.094730049 -0500
>...
> @@ -2001,7 +2000,6 @@
>  static int ip2_tiocmget(struct tty_struct *tty, struct file *file)
>  {
>  	i2ChanStrPtr pCh = DevTable[tty->index];
> -	wait_queue_t wait;
>  
>  	if (pCh == NULL)
>  		return -ENODEV;
> @@ -2018,6 +2016,8 @@
>  		/\/\|=mhw=|\/\/			*/
>  
>  #ifdef	ENABLE_DSSNOW
> +	wait_queue_t wait;
> +
>  	i2QueueCommands(PTYPE_BYPASS, pCh, 100, 1, CMD_DSS_NOW);
>  
>  	init_waitqueue_entry(&wait, current);

If someone will ever define ENABLE_DSSNOW your change broke compilation 
with gcc 2.95 (variable declaration mixed with code).

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] ip2: fix compile warnings

[Jim Nelson]

Adrian Bunk wrote:
> On Fri, Dec 17, 2004 at 03:47:13PM -0600, james4765@verizon.net wrote:
> 
> 
>>This fixes the following compile errors in the ip2 and ip2main drivers:
>>
>>  CC      drivers/char/ip2main.o
>>drivers/char/ip2main.c:470: warning: initialization from incompatible pointer type
>>drivers/char/ip2main.c: In function `ip2_tiocmget':
>>drivers/char/ip2main.c:2004: warning: unused variable `wait'
>>drivers/char/ip2/i2lib.c: At top level:
>>drivers/char/ip2/i2cmd.c:142: warning: `ct89' defined but not used
>>drivers/char/ip2main.c:205: warning: `set_modem_info' declared `static' but never defined
>>drivers/char/ip2/i2ellis.c:108: warning: `iiEllisCleanup' defined but not used
>>...
> 
> 
> This are not errors - they are only warnings.
>  
> 
>>--- linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c	2004-12-03 16:55:03.000000000 -0500
>>+++ linux-2.6.10-rc3-mm1/drivers/char/ip2main.c	2004-12-17 16:24:24.094730049 -0500
>>...
>>@@ -2001,7 +2000,6 @@
>> static int ip2_tiocmget(struct tty_struct *tty, struct file *file)
>> {
>> 	i2ChanStrPtr pCh = DevTable[tty->index];
>>-	wait_queue_t wait;
>> 
>> 	if (pCh == NULL)
>> 		return -ENODEV;
>>@@ -2018,6 +2016,8 @@
>> 		/\/\|=mhw=|\/\/			*/
>> 
>> #ifdef	ENABLE_DSSNOW
>>+	wait_queue_t wait;
>>+
>> 	i2QueueCommands(PTYPE_BYPASS, pCh, 100, 1, CMD_DSS_NOW);
>> 
>> 	init_waitqueue_entry(&wait, current);
> 
> 
> If someone will ever define ENABLE_DSSNOW your change broke compilation 
> with gcc 2.95 (variable declaration mixed with code).
> 

How about doing something like this:

#ifdef ENABLE_DSSNOW
#define DSSNOW_ENABLED 1
#else
#define DSSNOW_ENABLED 0
#endif

-blah-

static int ip2_tiocmget(struct tty_struct *tty, struct file *file)
{
	i2ChanStrPtr pCh = DevTable[tty->index];

	if (pCh == NULL)
		return -ENODEV;

if (DSSNOW_ENABLED) {
	if (ip2_tiocmget_dssnow() == -EINTR) return -EINTR;
	}

	return  ((pCh->dataSetOut & I2_RTS) ? TIOCM_RTS : 0)
	      | ((pCh->dataSetOut & I2_DTR) ? TIOCM_DTR : 0)
	      | ((pCh->dataSetIn  & I2_DCD) ? TIOCM_CAR : 0)
	      | ((pCh->dataSetIn  & I2_RI)  ? TIOCM_RNG : 0)
	      | ((pCh->dataSetIn  & I2_DSR) ? TIOCM_DSR : 0)
	      | ((pCh->dataSetIn  & I2_CTS) ? TIOCM_CTS : 0);
}

/*
	FIXME - the following code is causing a NULL pointer dereference in
	2.3.51 in an interrupt handler.  It's suppose to prompt the board
	to return the DSS signal status immediately.  Why doesn't it do
	the same thing in 2.2.14?
*/

/*	This thing is still busted in the 1.2.12 driver on 2.4.x
	and even hoses the serial console so the oops can be trapped.
		/\/\|=mhw=|\/\/			*/

static int ip2_tiocmget_dssnow (void)
{
	wait_queue_t wait;

	i2QueueCommands(PTYPE_BYPASS, pCh, 100, 1, CMD_DSS_NOW);

	init_waitqueue_entry(&wait, current);
	add_wait_queue(&pCh->dss_now_wait, &wait);
	set_current_state( TASK_INTERRUPTIBLE );

	serviceOutgoingFifo( pCh->pMyBord );

	schedule();

	set_current_state( TASK_RUNNING );
	remove_wait_queue(&pCh->dss_now_wait, &wait);

	if (signal_pending(current)) {
		return -EINTR;
	}

	return 0;
}

> cu
> Adrian
> 

Re: [PATCH] ip2: fix compile warnings

[Jim Nelson]

Randy.Dunlap wrote:
> james4765@verizon.net wrote:
> 
>> This fixes the following compile errors in the ip2 and ip2main drivers:
>>
>>   CC      drivers/char/ip2main.o
>> drivers/char/ip2main.c:470: warning: initialization from incompatible 
>> pointer type
> 
> 
> 
>> diff -urN --exclude='*~' 
>> linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c 
>> linux-2.6.10-rc3-mm1/drivers/char/ip2main.c
>> --- linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c    2004-12-03 
>> 16:55:03.000000000 -0500
>> +++ linux-2.6.10-rc3-mm1/drivers/char/ip2main.c    2004-12-17 
>> 16:24:24.094730049 -0500
>> @@ -467,7 +466,7 @@
>>  static struct tty_operations ip2_ops = {
>>      .open            = ip2_open,
>>      .close           = ip2_close,
>> -    .write           = ip2_write,
>> +    .write           = (void *) ip2_write,
>>      .put_char        = ip2_putchar,
>>      .flush_chars     = ip2_flush_chars,
>>      .write_room      = ip2_write_room,
> 
> 
> The write() prototype in tty_operations is:
>     int  (*write)(struct tty_struct * tty,
>               const unsigned char *buf, int count);
> 
> Somehow the cast does eliminate the compiler warning (and give
> a false sense of correctness).
> 
> However, ip2main.c::ip2_write() should be modified like so:
> 
> static int
> ip2_write( PTTY tty, const unsigned char *pData, int count)
> 
> and drop the cast and fix the ip2_write comment (drop old arg 2),
> and fix the ip2_write() prototype.
> But then you (someone) will have to decide how to handle the
> dropped <user> parameter when calling i2Output()...
> I don't know the answer to that.
> I just changed <user> to 0 to get a clean build of ip2main.o,
> but ip2/i2lib.c still needs some work.
> 

Sorry for the constant n00b questions, but:

Is there anything outside the kernel that could call tty_operations.write?

drivers/input/serio/serport.c uses: (as an example)

static int serport_serio_write(struct serio *serio, unsigned char data)
{
	struct serport *serport = serio->port_data;
	return -(serport->tty->driver->write(serport->tty, &data, 1) != 1);
}

I'm guessing that something does copy_from_user() before tty_operations.write is 
called, but I don't know quite what that is.

Can anyone to point me in the direction of where the user/kernel interface for tty 
devices is?

Given the way this is set up -

int  (*write)(struct tty_struct * tty, const unsigned char *buf, int count);

vs.

ip2_write( PTTY tty, int user, const unsigned char *pData, int count)

I don't even know if the driver would work - I think you'd have serious problems 
as it tries to dereference a pointer that is half-integer.

Am I reading this wrong?

Jim

Re: [PATCH] ip2: fix compile warnings

[Adrian Bunk]

On Fri, Dec 17, 2004 at 10:16:24PM -0500, Jim Nelson wrote:
> 
> How about doing something like this:
> 
> #ifdef ENABLE_DSSNOW
> #define DSSNOW_ENABLED 1
> #else
> #define DSSNOW_ENABLED 0
> #endif
> 
> -blah-
> 
> static int ip2_tiocmget(struct tty_struct *tty, struct file *file)
> {
> 	i2ChanStrPtr pCh = DevTable[tty->index];
> 
> 	if (pCh == NULL)
> 		return -ENODEV;
> 
> if (DSSNOW_ENABLED) {
> 	if (ip2_tiocmget_dssnow() == -EINTR) return -EINTR;
> 	}
> 
> 	return  ((pCh->dataSetOut & I2_RTS) ? TIOCM_RTS : 0)
> 	      | ((pCh->dataSetOut & I2_DTR) ? TIOCM_DTR : 0)
> 	      | ((pCh->dataSetIn  & I2_DCD) ? TIOCM_CAR : 0)
> 	      | ((pCh->dataSetIn  & I2_RI)  ? TIOCM_RNG : 0)
> 	      | ((pCh->dataSetIn  & I2_DSR) ? TIOCM_DSR : 0)
> 	      | ((pCh->dataSetIn  & I2_CTS) ? TIOCM_CTS : 0);
> }
> 
> /*
> 	FIXME - the following code is causing a NULL pointer dereference in
> 	2.3.51 in an interrupt handler.  It's suppose to prompt the board
> 	to return the DSS signal status immediately.  Why doesn't it do
> 	the same thing in 2.2.14?
> */
> 
> /*	This thing is still busted in the 1.2.12 driver on 2.4.x
> 	and even hoses the serial console so the oops can be trapped.
> 		/\/\|=mhw=|\/\/			*/
> 
> static int ip2_tiocmget_dssnow (void)
> {
>...

The real issue is the FIXME in the comment.

Such a patch would remove the warning but wouldn't fix the underlying 
real issue.

Fixing warnings is usually not a bad idea, but in this case I'd say the 
warning could stay until somebody fixes the real issue in the code.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] ip2: fix compile warnings

[Randy.Dunlap]

Jim Nelson wrote:
> Randy.Dunlap wrote:
> 
>> james4765@verizon.net wrote:
>>
>>> This fixes the following compile errors in the ip2 and ip2main drivers:
>>>
>>>   CC      drivers/char/ip2main.o
>>> drivers/char/ip2main.c:470: warning: initialization from incompatible 
>>> pointer type
>>
>>
>>> diff -urN --exclude='*~' 
>>> linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c 
>>> linux-2.6.10-rc3-mm1/drivers/char/ip2main.c
>>> --- linux-2.6.10-rc3-mm1-original/drivers/char/ip2main.c    
>>> 2004-12-03 16:55:03.000000000 -0500
>>> +++ linux-2.6.10-rc3-mm1/drivers/char/ip2main.c    2004-12-17 
>>> 16:24:24.094730049 -0500
>>> @@ -467,7 +466,7 @@
>>>  static struct tty_operations ip2_ops = {
>>>      .open            = ip2_open,
>>>      .close           = ip2_close,
>>> -    .write           = ip2_write,
>>> +    .write           = (void *) ip2_write,
>>>      .put_char        = ip2_putchar,
>>>      .flush_chars     = ip2_flush_chars,
>>>      .write_room      = ip2_write_room,
>>
>>
>> The write() prototype in tty_operations is:
>>     int  (*write)(struct tty_struct * tty,
>>               const unsigned char *buf, int count);
>>
>> Somehow the cast does eliminate the compiler warning (and give
>> a false sense of correctness).
>>
>> However, ip2main.c::ip2_write() should be modified like so:
>>
>> static int
>> ip2_write( PTTY tty, const unsigned char *pData, int count)
>>
>> and drop the cast and fix the ip2_write comment (drop old arg 2),
>> and fix the ip2_write() prototype.
>> But then you (someone) will have to decide how to handle the
>> dropped <user> parameter when calling i2Output()...
>> I don't know the answer to that.
>> I just changed <user> to 0 to get a clean build of ip2main.o,
>> but ip2/i2lib.c still needs some work.
>>
> 
> Sorry for the constant n00b questions, but:
> 
> Is there anything outside the kernel that could call tty_operations.write?

Sorry, I don't know the path to get to that code.

> drivers/input/serio/serport.c uses: (as an example)
> 
> static int serport_serio_write(struct serio *serio, unsigned char data)
> {
>     struct serport *serport = serio->port_data;
>     return -(serport->tty->driver->write(serport->tty, &data, 1) != 1);
> }
> 
> I'm guessing that something does copy_from_user() before 
> tty_operations.write is called, but I don't know quite what that is.
> 
> Can anyone to point me in the direction of where the user/kernel 
> interface for tty devices is?
> 
> Given the way this is set up -
> 
> int  (*write)(struct tty_struct * tty, const unsigned char *buf, int 
> count);
> 
> vs.
> 
> ip2_write( PTTY tty, int user, const unsigned char *pData, int count)
> 
> I don't even know if the driver would work - I think you'd have serious 
> problems as it tries to dereference a pointer that is half-integer.

I didn't quite get the "half-integer" part...
Unless you mean that the function interface is just broken.
Yes, it is.

> Am I reading this wrong?

It looks to me like the Kconfig entry for CONFIG_COMPUTONE
should just use BROKEN instead of BROKEN_ON_SMP since its
.write function interface hasn't been updated.

-- 
~Randy