Some cleanup patches for: '...lvalues is deprecated'

Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 3470 bytes --]

Hi Marcelo,

Please appolgies first for wrong presentation of previous post (that was the first and certainly the last time that I used the 
'forwarding' option of this webmail interface :( ).

Here are some backport to clean up some warning of type: use of cast experssion
as lvalues is deprecated.
--- linux-2.4.27-rc2-pa4mm/kernel/sysctl.c.Orig	2004-06-29 09:03:42.000000000 +0200
+++ linux-2.4.27-rc2-pa4mm/kernel/sysctl.c	2004-06-29 10:10:31.588030256 +0200
@@ -890,7 +890,7 @@
  				if (!isspace(c))
  					break;
  				left--;
-				((char *) buffer)++;
+				buffer += sizeof(char);
  			}
  			if (!left)
  				break;
@@ -1043,7 +1043,7 @@
  				if (!isspace(c))
  					break;
  				left--;
-				((char *) buffer)++;
+				buffer += sizeof(char);
  			}
  			if (!left)
  				break;
@@ -1144,7 +1144,7 @@
  				if (!isspace(c))
  					break;
  				left--;
-				((char *) buffer)++;
+				buffer += sizeof(char);
  			}
  			if (!left)
  				break;
=========><=========
--- linux-2.4.27-rc2-pa4mm/fs/readdir.c.Orig	2004-06-29 11:18:46.636488264 +0200
+++ linux-2.4.27-rc2-pa4mm/fs/readdir.c	2004-06-29 11:25:40.281604648 +0200
@@ -264,7 +264,7 @@
  	put_user(reclen, &dirent->d_reclen);
  	copy_to_user(dirent->d_name, name, namlen);
  	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (void *)dirent + reclen;
  	buf->current_dir = dirent;
  	buf->count -= reclen;
  	return 0;
@@ -347,7 +347,7 @@
  	copy_to_user(dirent, &d, NAME_OFFSET(&d));
  	copy_to_user(dirent->d_name, name, namlen);
  	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (void *)dirent + reclen;
  	buf->current_dir = dirent;
  	buf->count -= reclen;
  	return 0;
=========><=========
--- linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c.Orig	2004-06-29 10:47:31.901491304 +0200
+++ linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c	2004-06-29 11:13:31.846343640 +0200
@@ -1877,7 +1877,10 @@
         font length must be multiple of 256, at least. And 256 is multiple
         of 4 */
      k = 0;
-    while (p > new_data) k += *--(u32 *)p;
+    while (p > new_data) {
+        p = (u8 *)((u32 *)p - 1);
+        k += *(u32 *)p;
+    }
      FNTSUM(new_data) = k;
      /* Check if the same font is on some other console already */
      for (i = 0; i < MAX_NR_CONSOLES; i++) {
=========><=========
--- linux-2.4.27-rc2-pa4mm/lib/crc32.c.Orig	2004-06-29 11:29:31.721420448 +0200
+++ linux-2.4.27-rc2-pa4mm/lib/crc32.c	2004-06-29 11:36:19.964358088 +0200
@@ -99,7 +99,9 @@
  	/* Align it */
  	if(unlikely(((long)b)&3 && len)){
  		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
  		} while ((--len) && ((long)b)&3 );
  	}
  	if(likely(len >= 4)){
@@ -120,7 +122,9 @@
  	/* And the last few bytes */
  	if(len){
  		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
  		} while (--len);
  	}

@@ -200,7 +204,9 @@
  	/* Align it */
  	if(unlikely(((long)b)&3 && len)){
  		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
  		} while ((--len) && ((long)b)&3 );
  	}
  	if(likely(len >= 4)){
@@ -221,7 +227,9 @@
  	/* And the last few bytes */
  	if(len){
  		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
  		} while (--len);
  	}
  	return __be32_to_cpu(crc);
=========><=========

hth,
     Joel

PS: because of bad wrapping pb with mail interface I also join original
files

[-- Attachment #2: k-2.4.27-rc2_crc32.c.diff --]
[-- Type: text/plain, Size: 1002 bytes --]

--- linux-2.4.27-rc2-pa4mm/lib/crc32.c.Orig	2004-06-29 11:29:31.721420448 +0200
+++ linux-2.4.27-rc2-pa4mm/lib/crc32.c	2004-06-29 11:36:19.964358088 +0200
@@ -99,7 +99,9 @@
 	/* Align it */
 	if(unlikely(((long)b)&3 && len)){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
 		} while ((--len) && ((long)b)&3 );
 	}
 	if(likely(len >= 4)){
@@ -120,7 +122,9 @@
 	/* And the last few bytes */
 	if(len){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
 		} while (--len);
 	}
 
@@ -200,7 +204,9 @@
 	/* Align it */
 	if(unlikely(((long)b)&3 && len)){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
 		} while ((--len) && ((long)b)&3 );
 	}
 	if(likely(len >= 4)){
@@ -221,7 +227,9 @@
 	/* And the last few bytes */
 	if(len){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *p = (u8 *)b;
+			DO_CRC(*p++);
+			b = (void *)p;
 		} while (--len);
 	}
 	return __be32_to_cpu(crc);

[-- Attachment #3: k-2.4.27-rc2_fbcon.c.diff --]
[-- Type: text/plain, Size: 579 bytes --]

--- linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c.Orig	2004-06-29 10:47:31.901491304 +0200
+++ linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c	2004-06-29 11:13:31.846343640 +0200
@@ -1877,7 +1877,10 @@
        font length must be multiple of 256, at least. And 256 is multiple
        of 4 */
     k = 0;
-    while (p > new_data) k += *--(u32 *)p;
+    while (p > new_data) {
+        p = (u8 *)((u32 *)p - 1);
+        k += *(u32 *)p;
+    }
     FNTSUM(new_data) = k;
     /* Check if the same font is on some other console already */
     for (i = 0; i < MAX_NR_CONSOLES; i++) {

[-- Attachment #4: k-2.4.27-rc2_readdir.c.diff --]
[-- Type: text/plain, Size: 717 bytes --]

--- linux-2.4.27-rc2-pa4mm/fs/readdir.c.Orig	2004-06-29 11:18:46.636488264 +0200
+++ linux-2.4.27-rc2-pa4mm/fs/readdir.c	2004-06-29 11:25:40.281604648 +0200
@@ -264,7 +264,7 @@
 	put_user(reclen, &dirent->d_reclen);
 	copy_to_user(dirent->d_name, name, namlen);
 	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (void *)dirent + reclen;
 	buf->current_dir = dirent;
 	buf->count -= reclen;
 	return 0;
@@ -347,7 +347,7 @@
 	copy_to_user(dirent, &d, NAME_OFFSET(&d));
 	copy_to_user(dirent->d_name, name, namlen);
 	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (void *)dirent + reclen;
 	buf->current_dir = dirent;
 	buf->count -= reclen;
 	return 0;

[-- Attachment #5: k-2.4.27-rc2_sysctl.c.diff --]
[-- Type: text/plain, Size: 635 bytes --]

--- linux-2.4.27-rc2-pa4mm/kernel/sysctl.c.Orig	2004-06-29 09:03:42.000000000 +0200
+++ linux-2.4.27-rc2-pa4mm/kernel/sysctl.c	2004-06-29 10:10:31.588030256 +0200
@@ -890,7 +890,7 @@
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				buffer += sizeof(char);
 			}
 			if (!left)
 				break;
@@ -1043,7 +1043,7 @@
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				buffer += sizeof(char);
 			}
 			if (!left)
 				break;
@@ -1144,7 +1144,7 @@
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				buffer += sizeof(char);
 			}
 			if (!left)
 				break;

Re: Some cleanup patches for: '...lvalues is deprecated'

[Vojtech Pavlik]

On Sat, Jul 03, 2004 at 12:53:21PM +0000, Joel Soete wrote:
> Hi Marcelo,
> 
> Please appolgies first for wrong presentation of previous post (that was 
> the first and certainly the last time that I used the 'forwarding' option 
> of this webmail interface :( ).
> 
> Here are some backport to clean up some warning of type: use of cast 
> experssion
> as lvalues is deprecated.
> --- linux-2.4.27-rc2-pa4mm/kernel/sysctl.c.Orig	2004-06-29 
> 09:03:42.000000000 +0200
> +++ linux-2.4.27-rc2-pa4mm/kernel/sysctl.c	2004-06-29 
> 10:10:31.588030256 +0200
> @@ -890,7 +890,7 @@
>  				if (!isspace(c))
>  					break;
>  				left--;
> -				((char *) buffer)++;
> +				buffer += sizeof(char);

This (although correct in the end) is a wrong thing to do.

It seems to look like the intention is to move the pointer by a char's
size, however your change is equivalent to:

	buffer += 1;

And if buffer wasn't void*, which it fortunately is, it would, unlike
the older construction, move the pointer by a different size.

So just use

	buffer++;

here, and the intent is then clear.

> --- linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c.Orig	2004-06-29 
> 10:47:31.901491304 +0200
> +++ linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c	2004-06-29 
> 11:13:31.846343640 +0200
> @@ -1877,7 +1877,10 @@
>         font length must be multiple of 256, at least. And 256 is multiple
>         of 4 */
>      k = 0;
> -    while (p > new_data) k += *--(u32 *)p;
> +    while (p > new_data) {
> +        p = (u8 *)((u32 *)p - 1);
> +        k += *(u32 *)p;
> +    }


How about

	p -= 4;
	k += *(u32 *)p;

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

Thanks first for your attention ;)

Vojtech Pavlik wrote:
> On Sat, Jul 03, 2004 at 12:53:21PM +0000, Joel Soete wrote:
> 
>>Hi Marcelo,
>>
>>Please appolgies first for wrong presentation of previous post (that was 
>>the first and certainly the last time that I used the 'forwarding' option 
>>of this webmail interface :( ).
>>
>>Here are some backport to clean up some warning of type: use of cast 
>>experssion
>>as lvalues is deprecated.
>>--- linux-2.4.27-rc2-pa4mm/kernel/sysctl.c.Orig	2004-06-29 
>>09:03:42.000000000 +0200
>>+++ linux-2.4.27-rc2-pa4mm/kernel/sysctl.c	2004-06-29 
>>10:10:31.588030256 +0200
>>@@ -890,7 +890,7 @@
>> 				if (!isspace(c))
>> 					break;
>> 				left--;
>>-				((char *) buffer)++;
>>+				buffer += sizeof(char);
> 
> 
> This (although correct in the end) is a wrong thing to do.
> 
> It seems to look like the intention is to move the pointer by a char's
> size, however your change is equivalent to:
> 
> 	buffer += 1;
> 
> And if buffer wasn't void*, which it fortunately is, it would, unlike
> the older construction, move the pointer by a different size.
> 
Very interesting but well I am not enough fluent in C to understand this fine detail :(
Can you explain me by an example (let say a long*) what would did "((char *) buffer)++;" versus "buffer += sizeof(char);"
(Don't worry, if don't have time, I will perfectly understand and will experiment by myself)

> So just use
> 
> 	buffer++;
> 
> here, and the intent is then clear.
> 
Yes ;)
> 
>>--- linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c.Orig	2004-06-29 
>>10:47:31.901491304 +0200
>>+++ linux-2.4.27-rc2-pa4mm/drivers/video/fbcon.c	2004-06-29 
>>11:13:31.846343640 +0200
>>@@ -1877,7 +1877,10 @@
>>        font length must be multiple of 256, at least. And 256 is multiple
>>        of 4 */
>>     k = 0;
>>-    while (p > new_data) k += *--(u32 *)p;
>>+    while (p > new_data) {
>>+        p = (u8 *)((u32 *)p - 1);
>>+        k += *(u32 *)p;
>>+    }
> 
> 
> 
> How about
> 
> 	p -= 4;
> 	k += *(u32 *)p;
> 
:)

Thanks again,
	Joel

Re: Some cleanup patches for: '...lvalues is deprecated'

[Vojtech Pavlik]

On Sat, Jul 03, 2004 at 09:39:24PM +0000, Joel Soete wrote:

> Very interesting but well I am not enough fluent in C to understand this 
> fine detail :(
> Can you explain me by an example (let say a long*) what would did "((char 
> *) buffer)++;" versus "buffer += sizeof(char);"
> (Don't worry, if don't have time, I will perfectly understand and will 
> experiment by myself)

Ok. Let's assume

	int *buffer;

just for this example.

	((char *) buffer)++;

increments buffer by 1, while

	buffer += sizeof(char);

increments buffer by 4, because an int* is always
increased/decreased by multiples of sizeof(int).

So

	buffer += 2;

would increment the pointer by 8.

Similarly for other pointer types.

> >So just use
> >
> >	buffer++;
> >
> >here, and the intent is then clear.
> >
> Yes ;)

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: Some cleanup patches for: '...lvalues is deprecated'

[Daniel Jacobowitz]

On Sat, Jul 03, 2004 at 10:56:21PM +0200, Vojtech Pavlik wrote:
> On Sat, Jul 03, 2004 at 12:53:21PM +0000, Joel Soete wrote:
> > Hi Marcelo,
> > 
> > Please appolgies first for wrong presentation of previous post (that was 
> > the first and certainly the last time that I used the 'forwarding' option 
> > of this webmail interface :( ).
> > 
> > Here are some backport to clean up some warning of type: use of cast 
> > experssion
> > as lvalues is deprecated.
> > --- linux-2.4.27-rc2-pa4mm/kernel/sysctl.c.Orig	2004-06-29 
> > 09:03:42.000000000 +0200
> > +++ linux-2.4.27-rc2-pa4mm/kernel/sysctl.c	2004-06-29 
> > 10:10:31.588030256 +0200
> > @@ -890,7 +890,7 @@
> >  				if (!isspace(c))
> >  					break;
> >  				left--;
> > -				((char *) buffer)++;
> > +				buffer += sizeof(char);
> 
> This (although correct in the end) is a wrong thing to do.
> 
> It seems to look like the intention is to move the pointer by a char's
> size, however your change is equivalent to:
> 
> 	buffer += 1;
> 
> And if buffer wasn't void*, which it fortunately is, it would, unlike
> the older construction, move the pointer by a different size.
> 
> So just use
> 
> 	buffer++;
> 
> here, and the intent is then clear.

Except C does not actually allow incrementing a void pointer, since
void does not have a size.  You can't do arithmetic on one either.  GNU
C allows this as an extension.

It's actually this, IIRC:
  buffer = ((char *) buffer) + 1;

-- 
Daniel Jacobowitz

Re: Some cleanup patches for: '...lvalues is deprecated'

[David Vrabel]

Daniel Jacobowitz wrote:
> 
> Except C does not actually allow incrementing a void pointer, since
> void does not have a size.  You can't do arithmetic on one either.  GNU
> C allows this as an extension.

GCC seems to have a whole bunch of ill thought out extensions like this.

David Vrabel
-- 
David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

Hello Daniel,

> > So just use
> > 
> > 	buffer++;
> > 
> > here, and the intent is then clear.
> 
> Except C does not actually allow incrementing a void pointer, since
> void does not have a size.
That make better sense to me because aifair a void * was foreseen to pass
any kind of type * as actual parameter?
(So as far as I understand, the aritthm pointer sould be dynamic for the
best 'natural' behaviour?)

>   You can't do arithmetic on one either.  GNU
> C allows this as an extension.
> 
> It's actually this, IIRC:
>   buffer = ((char *) buffer) + 1;
> 
Many thanks,
    Joel


---------------------------------------------------------------------------
Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de faire
le pas!
http://reg.tiscali.be/default.asp?lg=fr

Re: Some cleanup patches for: '...lvalues is deprecated'

[Marcelo Tosatti]

On Mon, Jul 05, 2004 at 01:59:21PM +0200, Joel Soete wrote:
> Hello Daniel,
> 
> > > So just use
> > >
> > > 	buffer++;
> > >
> > > here, and the intent is then clear.
> >
> > Except C does not actually allow incrementing a void pointer, since
> > void does not have a size.
> That make better sense to me because aifair a void * was foreseen to pass
> any kind of type * as actual parameter?
> (So as far as I understand, the aritthm pointer sould be dynamic for the
> best 'natural' behaviour?)
> 
> >   You can't do arithmetic on one either.  GNU
> > C allows this as an extension.
> >
> > It's actually this, IIRC:
> >   buffer = ((char *) buffer) + 1;

Joel, 

It seems the current code is working perfectly, generating correct
asm code. 

Could you come up with a good enough reason to do this cleanup (as far as 
I am concerned) in 2.4.x series?

Thanks

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

Marcelo,

Thanks first for your attention.
Sorry also for delaying this works but I was a bit busy elsewhere.

> -- Original Message --
> Date: Tue, 27 Jul 2004 09:54:32 -0300
> From: Marcelo Tosatti <marcelo.tosatti@cyclades.com>
> To: Joel Soete <soete.joel@tiscali.be>
> Cc: Daniel Jacobowitz <dan@debian.org>,
> 	Vojtech Pavlik <vojtech@suse.cz>,
> 	Linux Kernel <linux-kernel@vger.kernel.org>
> Subject: Re: Some cleanup patches for: '...lvalues is deprecated'
> 
> 
> On Mon, Jul 05, 2004 at 01:59:21PM +0200, Joel Soete wrote:
> > Hello Daniel,
> > 
> > > > So just use
> > > >
> > > > 	buffer++;
> > > >
> > > > here, and the intent is then clear.

> > >
> > > Except C does not actually allow incrementing a void pointer, since
> > > void does not have a size.
> > That make better sense to me because aifair a void * was foreseen to
pass
> > any kind of type * as actual parameter?
> > (So as far as I understand, the aritthm pointer sould be dynamic for
the
> > best 'natural' behaviour?)
> > 
> > >   You can't do arithmetic on one either.  GNU
> > > C allows this as an extension.
> > >
> > > It's actually this, IIRC:
> > >   buffer = ((char *) buffer) + 1;
> 
> Joel, 
> 
> It seems the current code is working perfectly, generating correct
> asm code. 
> 
> Could you come up with a good enough reason to do this cleanup (as far
as
> 
> I am concerned) in 2.4.x series?
> 
My first attention was to cleanup some warning of type "use of cast expression
as lvalue is deprecated"
with gcc-3.3.4. But afaik, right now, there are just few warning which didn't
break the asm code.

I will try to come back asap with a better solution (at least I hope ;)
)

Thanks again,
   Joel

---------------------------------------------------------------------------
Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de faire
le pas!
http://reg.tiscali.be/default.asp?lg=fr

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

Marcelo,

Thanks first for your attention.
Sorry also for delaying this works but I was a bit busy elsewhere.

> -- Original Message --
> Date: Tue, 27 Jul 2004 09:54:32 -0300
> From: Marcelo Tosatti <marcelo.tosatti@cyclades.com>
> To: Joel Soete <soete.joel@tiscali.be>
> Cc: Daniel Jacobowitz <dan@debian.org>,
> 	Vojtech Pavlik <vojtech@suse.cz>,
> 	Linux Kernel <linux-kernel@vger.kernel.org>
> Subject: Re: Some cleanup patches for: '...lvalues is deprecated'
> 
> 
> On Mon, Jul 05, 2004 at 01:59:21PM +0200, Joel Soete wrote:
> > Hello Daniel,
> > 
> > > > So just use
> > > >
> > > > 	buffer++;
> > > >
> > > > here, and the intent is then clear.

> > >
> > > Except C does not actually allow incrementing a void pointer, since
> > > void does not have a size.
> > That make better sense to me because aifair a void * was foreseen to
pass
> > any kind of type * as actual parameter?
> > (So as far as I understand, the aritthm pointer sould be dynamic for
the
> > best 'natural' behaviour?)
> > 
> > >   You can't do arithmetic on one either.  GNU
> > > C allows this as an extension.
> > >
> > > It's actually this, IIRC:
> > >   buffer = ((char *) buffer) + 1;
> 
> Joel, 
> 
> It seems the current code is working perfectly, generating correct
> asm code. 
> 
> Could you come up with a good enough reason to do this cleanup (as far
as
> 
> I am concerned) in 2.4.x series?
> 
My first attention was to cleanup some warning of type "use of cast expression
as lvalue is deprecated"
with gcc-3.3.4. But afaik, right now, there are just few warning which didn't
break the asm code.

I will try to come back asap with a better solution (at least I hope ;)
)

Thanks again,
   Joel

---------------------------------------------------------------------------
Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de faire
le pas!
http://reg.tiscali.be/default.asp?lg=fr

Re: Some cleanup patches for: '...lvalues is deprecated'

[Jon Oberheide]

[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]

On Tue, 2004-07-27 at 17:59 +0200, Joel Soete wrote:
> Marcelo,
> 
> Thanks first for your attention.
> Sorry also for delaying this works but I was a bit busy elsewhere.
> 
> > -- Original Message --
> > Date: Tue, 27 Jul 2004 09:54:32 -0300
> > From: Marcelo Tosatti <marcelo.tosatti@cyclades.com>
> > To: Joel Soete <soete.joel@tiscali.be>
> > Cc: Daniel Jacobowitz <dan@debian.org>,
> > 	Vojtech Pavlik <vojtech@suse.cz>,
> > 	Linux Kernel <linux-kernel@vger.kernel.org>
> > Subject: Re: Some cleanup patches for: '...lvalues is deprecated'
> > 
> > 
> > On Mon, Jul 05, 2004 at 01:59:21PM +0200, Joel Soete wrote:
> > > Hello Daniel,
> > > 
> > > > > So just use
> > > > >
> > > > > 	buffer++;
> > > > >
> > > > > here, and the intent is then clear.
> 
> > > >
> > > > Except C does not actually allow incrementing a void pointer, since
> > > > void does not have a size.
> > > That make better sense to me because aifair a void * was foreseen to
> pass
> > > any kind of type * as actual parameter?
> > > (So as far as I understand, the aritthm pointer sould be dynamic for
> the
> > > best 'natural' behaviour?)
> > > 
> > > >   You can't do arithmetic on one either.  GNU
> > > > C allows this as an extension.
> > > >
> > > > It's actually this, IIRC:
> > > >   buffer = ((char *) buffer) + 1;
> > 
> > Joel, 
> > 
> > It seems the current code is working perfectly, generating correct
> > asm code. 
> > 
> > Could you come up with a good enough reason to do this cleanup (as far
> as
> > 
> > I am concerned) in 2.4.x series?
> > 
> My first attention was to cleanup some warning of type "use of cast expression
> as lvalue is deprecated"
> with gcc-3.3.4. But afaik, right now, there are just few warning which didn't
> break the asm code.

FYI, lvalue casts are treated as errors in gcc 3.5.

Regards,
Jon Oberheide

-- 
Jon Oberheide <jon@oberheide.org>
GnuPG Key: 1024D/F47C17FE
Fingerprint: B716 DA66 8173 6EDD 28F6  F184 5842 1C89 F47C 17FE

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

Hello *,

>
> FYI, lvalue casts are treated as errors in gcc 3.5.
>
According to this kind remark, I think so that following attachement patches
would be interesting.
Thanks for all relevant remarks to help me to make stuff cleaner.

Joel

PS: I don't yet review the lib/crc32.c (sorry I need more effort to review)


---------------------------------------------------------------------------
Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de faire
le pas!
http://reg.tiscali.be/default.asp?lg=fr





[-- Attachment #2: drivers-video-fbcon.diff --]
[-- Type: application/octet-stream, Size: 547 bytes --]

--- linux-2.4.27-rc3-pa6mm/drivers/video/fbcon.c.Orig	2004-06-29 10:47:31.000000000 +0200
+++ linux-2.4.27-rc3-pa6mm/drivers/video/fbcon.c	2004-07-30 09:21:43.295828520 +0200
@@ -1877,7 +1877,10 @@
        font length must be multiple of 256, at least. And 256 is multiple
        of 4 */
     k = 0;
-    while (p > new_data) k += *--(u32 *)p;
+    while (p > new_data) {
+	p -= 4;
+	k += *(u32 *)p;
+    }
     FNTSUM(new_data) = k;
     /* Check if the same font is on some other console already */
     for (i = 0; i < MAX_NR_CONSOLES; i++) {

[-- Attachment #3: fs-readdir.diff --]
[-- Type: application/octet-stream, Size: 769 bytes --]

--- linux-2.4.27-rc3-pa6mm/fs/readdir.c.Orig	2004-06-29 11:18:46.000000000 +0200
+++ linux-2.4.27-rc3-pa6mm/fs/readdir.c	2004-07-29 12:54:45.000000000 +0200
@@ -264,7 +264,7 @@
 	put_user(reclen, &dirent->d_reclen);
 	copy_to_user(dirent->d_name, name, namlen);
 	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (struct linux_dirent *)((char *)dirent + reclen);
 	buf->current_dir = dirent;
 	buf->count -= reclen;
 	return 0;
@@ -347,7 +347,7 @@
 	copy_to_user(dirent, &d, NAME_OFFSET(&d));
 	copy_to_user(dirent->d_name, name, namlen);
 	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (struct linux_dirent64 *)((char *)dirent + reclen);
 	buf->current_dir = dirent;
 	buf->count -= reclen;
 	return 0;

[-- Attachment #4: kernel-sysctl.diff --]
[-- Type: application/octet-stream, Size: 1191 bytes --]

--- linux-2.4.27-rc3-pa6mm/kernel/sysctl.c.Orig	2004-06-29 09:03:42.000000000 +0200
+++ linux-2.4.27-rc3-pa6mm/kernel/sysctl.c	2004-07-29 11:41:30.021094824 +0200
@@ -883,14 +883,15 @@
 	
 	for (; left && vleft--; i++, first=0) {
 		if (write) {
+			p = buffer;
 			while (left) {
 				char c;
-				if (get_user(c, (char *) buffer))
+				if (get_user(c, p))
 					return -EFAULT;
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				p++;
 			}
 			if (!left)
 				break;
@@ -1036,14 +1037,15 @@
 	
 	for (; left && vleft--; i++, min++, max++, first=0) {
 		if (write) {
+			p = buffer;
 			while (left) {
 				char c;
-				if (get_user(c, (char *) buffer))
+				if (get_user(c, p))
 					return -EFAULT;
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				p++;
 			}
 			if (!left)
 				break;
@@ -1137,14 +1139,15 @@
 	
 	for (; left && vleft--; i++, first=0) {
 		if (write) {
+			p = (char *)buffer;
 			while (left) {
 				char c;
-				if (get_user(c, (char *) buffer))
+				if (get_user(c, p))
 					return -EFAULT;
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				p++;
 			}
 			if (!left)
 				break;

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

Sorry but previous time included file were well readable and not this time?
too bad.

So here there are (cut and past? sorry in advance if bad wrapping will occurs)

--- linux-2.4.27-rc3-pa6mm/kernel/sysctl.c.Orig	2004-06-29 09:03:42.000000000
+0200
+++ linux-2.4.27-rc3-pa6mm/kernel/sysctl.c	2004-07-29 11:41:30.021094824
+0200
@@ -883,14 +883,15 @@
 	
 	for (; left && vleft--; i++, first=0) {
 		if (write) {
+			p = buffer;
 			while (left) {
 				char c;
-				if (get_user(c, (char *) buffer))
+				if (get_user(c, p))
 					return -EFAULT;
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				p++;
 			}
 			if (!left)
 				break;
@@ -1036,14 +1037,15 @@
 	
 	for (; left && vleft--; i++, min++, max++, first=0) {
 		if (write) {
+			p = buffer;
 			while (left) {
 				char c;
-				if (get_user(c, (char *) buffer))
+				if (get_user(c, p))
 					return -EFAULT;
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				p++;
 			}
 			if (!left)
 				break;
@@ -1137,14 +1139,15 @@
 	
 	for (; left && vleft--; i++, first=0) {
 		if (write) {
+			p = (char *)buffer;
 			while (left) {
 				char c;
-				if (get_user(c, (char *) buffer))
+				if (get_user(c, p))
 					return -EFAULT;
 				if (!isspace(c))
 					break;
 				left--;
-				((char *) buffer)++;
+				p++;
 			}
 			if (!left)
 				break;
--- linux-2.4.27-rc3-pa6mm/fs/readdir.c.Orig	2004-06-29 11:18:46.000000000
+0200
+++ linux-2.4.27-rc3-pa6mm/fs/readdir.c	2004-07-29 12:54:45.000000000 +0200
@@ -264,7 +264,7 @@
 	put_user(reclen, &dirent->d_reclen);
 	copy_to_user(dirent->d_name, name, namlen);
 	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (struct linux_dirent *)((char *)dirent + reclen);
 	buf->current_dir = dirent;
 	buf->count -= reclen;
 	return 0;
@@ -347,7 +347,7 @@
 	copy_to_user(dirent, &d, NAME_OFFSET(&d));
 	copy_to_user(dirent->d_name, name, namlen);
 	put_user(0, dirent->d_name + namlen);
-	((char *) dirent) += reclen;
+	dirent = (struct linux_dirent64 *)((char *)dirent + reclen);
 	buf->current_dir = dirent;
 	buf->count -= reclen;
 	return 0;
--- linux-2.4.27-rc3-pa6mm/drivers/video/fbcon.c.Orig	2004-06-29 10:47:31.000000000
+0200
+++ linux-2.4.27-rc3-pa6mm/drivers/video/fbcon.c	2004-07-30 09:21:43.295828520
+0200
@@ -1877,7 +1877,10 @@
        font length must be multiple of 256, at least. And 256 is multiple
        of 4 */
     k = 0;
-    while (p > new_data) k += *--(u32 *)p;
+    while (p > new_data) {
+	p -= 4;
+	k += *(u32 *)p;
+    }
     FNTSUM(new_data) = k;
     /* Check if the same font is on some other console already */
     for (i = 0; i < MAX_NR_CONSOLES; i++) {
=========><=========

> -- Original Message --
> Date: Fri, 30 Jul 2004 11:11:32 +0200
> From: "Joel Soete" <soete.joel@tiscali.be>
> Subject: Re: Some cleanup patches for: '...lvalues is deprecated'
> To: "Jon Oberheide" <jon@oberheide.org>
> Cc: "Marcelo Tosatti" <marcelo.tosatti@cyclades.com>,
>  "Daniel Jacobowitz" <dan@debian.org>,
>  "Vojtech Pavlik" <vojtech@suse.cz>,
>  "Linux Kernel" <linux-kernel@vger.kernel.org>
> 
> 
> Hello *,
> 
> >
> > FYI, lvalue casts are treated as errors in gcc 3.5.
> >
> According to this kind remark, I think so that following attachement patches
> would be interesting.
> Thanks for all relevant remarks to help me to make stuff cleaner.
> 
> Joel
> 
> PS: I don't yet review the lib/crc32.c (sorry I need more effort to review)
> 
> 
> ---------------------------------------------------------------------------
> Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de
faire
> le pas!
> http://reg.tiscali.be/default.asp?lg=fr
> 
> 
> 
> 
> 
> Attachment: drivers-video-fbcon.diff
> 
> 
> Attachment: fs-readdir.diff
> 
> 
> Attachment: kernel-sysctl.diff
> 


---------------------------------------------------------------------------
Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de faire
le pas!
http://reg.tiscali.be/default.asp?lg=fr

Re: Some cleanup patches for: '...lvalues is deprecated'

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

And finaly may be lib/crc32.c diff like:
--- linux-2.4.27-rc3-pa6mm/lib/crc32.c.Orig	2004-06-29 11:29:31.000000000
+0200
+++ linux-2.4.27-rc3-pa6mm/lib/crc32.c	2004-07-30 17:17:56.143095488 +0200
@@ -99,7 +99,9 @@
 	/* Align it */
 	if(unlikely(((long)b)&3 && len)){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while ((--len) && ((long)b)&3 );
 	}
 	if(likely(len >= 4)){
@@ -120,7 +122,9 @@
 	/* And the last few bytes */
 	if(len){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while (--len);
 	}

@@ -200,7 +204,9 @@
 	/* Align it */
 	if(unlikely(((long)b)&3 && len)){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while ((--len) && ((long)b)&3 );
 	}
 	if(likely(len >= 4)){
@@ -221,7 +227,9 @@
 	/* And the last few bytes */
 	if(len){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while (--len);
 	}
 	return __be32_to_cpu(crc);
=========><=========

(i prefer pb to avoid possible reading confusion with outer p parameter?
and replace usage of (void *) by (u32 *) as the actual b type)

Thanks for additional attention,
    Joel

---------------------------------------------------------------------------
Tiscali ADSL LIGHT, 19,95 EUR/mois pendant 6 mois, c'est le moment de faire
le pas!
http://reg.tiscali.be/default.asp?lg=fr





[-- Attachment #2: lib-crc32.c.diff --]
[-- Type: application/octet-stream, Size: 1010 bytes --]

--- linux-2.4.27-rc3-pa6mm/lib/crc32.c.Orig	2004-06-29 11:29:31.000000000 +0200
+++ linux-2.4.27-rc3-pa6mm/lib/crc32.c	2004-07-30 17:17:56.143095488 +0200
@@ -99,7 +99,9 @@
 	/* Align it */
 	if(unlikely(((long)b)&3 && len)){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while ((--len) && ((long)b)&3 );
 	}
 	if(likely(len >= 4)){
@@ -120,7 +122,9 @@
 	/* And the last few bytes */
 	if(len){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while (--len);
 	}
 
@@ -200,7 +204,9 @@
 	/* Align it */
 	if(unlikely(((long)b)&3 && len)){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while ((--len) && ((long)b)&3 );
 	}
 	if(likely(len >= 4)){
@@ -221,7 +227,9 @@
 	/* And the last few bytes */
 	if(len){
 		do {
-			DO_CRC(*((u8 *)b)++);
+			u8 *pb = (u8 *)b;
+			DO_CRC(*pb++);
+			b = (u32 *)pb;
 		} while (--len);
 	}
 	return __be32_to_cpu(crc);

Re: Some cleanup patches for: '...lvalues is deprecated'

[Mikael Pettersson]

On Fri, 30 Jul 2004 11:11:32 +0200, Joel Soete wrote:
>> FYI, lvalue casts are treated as errors in gcc 3.5.
>> 
>According to this kind remark, I think so that following attachement patc=
>hes
>would be interesting.

(cast-as-lvalue elimination patches omitted)

Did you know that there is a larger gcc-3.4 fixes patch:
<http://www.csd.uu.se/~mikpe/linux/patches/2.4/patch-gcc340-fixes-v4-2.4.27-rc3>
?

This patch handles all issues when using gcc-3.4 to compile
the current 2.4 kernel, of which cast-as-lvalue is just one.
The only difference, AFAIK, is that gcc-3.4 "merely" warns
about cast-as-lvalue while gcc-3.5 errors out on them.

All changes in the gcc-3.4 fixes patch are backports from
the 2.6 kernel, except in very few cases when 2.4 and 2.6
have diverged making slightly different fixes more appropriate
for 2.4.

The patch handles i386, x86-64, and ppc architecture code,
plus whatever drivers etc I've ever needed, plus drivers
etc other people have contributed or requested fixes for.
The only code I'm not considering is architecture code for
other architectures than i386/x86-64/ppc, since those are
the only ones I can compile and test.

/Mikael

Re: Some cleanup patches for: '...lvalues is deprecated'

[Adrian Bunk]

On Sat, Jul 31, 2004 at 10:41:53AM +0200, Mikael Pettersson wrote:
>...
> Did you know that there is a larger gcc-3.4 fixes patch:
> <http://www.csd.uu.se/~mikpe/linux/patches/2.4/patch-gcc340-fixes-v4-2.4.27-rc3>
> ?
> 
> This patch handles all issues when using gcc-3.4 to compile
> the current 2.4 kernel, of which cast-as-lvalue is just one.
> The only difference, AFAIK, is that gcc-3.4 "merely" warns
> about cast-as-lvalue while gcc-3.5 errors out on them.
> 
> All changes in the gcc-3.4 fixes patch are backports from
> the 2.6 kernel, except in very few cases when 2.4 and 2.6
> have diverged making slightly different fixes more appropriate
> for 2.4.
>...


BTW:

Please don't include the compiler.h part of your patch.
It's already reverted in -mm, and we're currently fixing the inlines
in 2.6 .

This was started after with this patch in 2.6 somewhere a required 
inlining did no longer occur (and a compile error is definitely better 
than a potential runtime problem).

Although fixing it correctly touches at about three dozen files, these 
are pretty straightforward patches (removing inlines or moving code 
inside files). After all these issues are sorted out in 2.6, the inline 
fixes could be backported to 2.4 .


> /Mikael

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: Some cleanup patches for: '...lvalues is deprecated'

[Mikael Pettersson]

On Sun, 1 Aug 2004 12:40:26 +0200, Adrian Bunk wrote:
>On Sat, Jul 31, 2004 at 10:41:53AM +0200, Mikael Pettersson wrote:
>>...
>> Did you know that there is a larger gcc-3.4 fixes patch:
>> <http://www.csd.uu.se/~mikpe/linux/patches/2.4/patch-gcc340-fixes-v4-2.4.27-rc3>
>> ?
>> 
>> This patch handles all issues when using gcc-3.4 to compile
>> the current 2.4 kernel, of which cast-as-lvalue is just one.
>> The only difference, AFAIK, is that gcc-3.4 "merely" warns
>> about cast-as-lvalue while gcc-3.5 errors out on them.
>> 
>> All changes in the gcc-3.4 fixes patch are backports from
>> the 2.6 kernel, except in very few cases when 2.4 and 2.6
>> have diverged making slightly different fixes more appropriate
>> for 2.4.
>>...
>
>
>BTW:
>
>Please don't include the compiler.h part of your patch.
>It's already reverted in -mm, and we're currently fixing the inlines
>in 2.6 .
>
>This was started after with this patch in 2.6 somewhere a required 
>inlining did no longer occur (and a compile error is definitely better 
>than a potential runtime problem).

Which one was that? DaveM wrote recently that they had eliminated
SPARC's save_flags/restore_flags-in-same-stack-frame requirement.

>Although fixing it correctly touches at about three dozen files, these 
>are pretty straightforward patches (removing inlines or moving code 
>inside files). After all these issues are sorted out in 2.6, the inline 
>fixes could be backported to 2.4 .

I'll consider attacking the inlining issues, after I've reviewed my
cast-as-lvalue and fastcall fixes -- I noticed that at least one fix
had changed in 2.6 since I first adapted it to 2.4.

/Mikael

Re: Some cleanup patches for: '...lvalues is deprecated'

[Adrian Bunk]

On Mon, Aug 02, 2004 at 01:34:26PM +0200, Mikael Pettersson wrote:
> On Sun, 1 Aug 2004 12:40:26 +0200, Adrian Bunk wrote:
> >
> >BTW:
> >
> >Please don't include the compiler.h part of your patch.
> >It's already reverted in -mm, and we're currently fixing the inlines
> >in 2.6 .
> >
> >This was started after with this patch in 2.6 somewhere a required 
> >inlining did no longer occur (and a compile error is definitely better 
> >than a potential runtime problem).
> 
> Which one was that? DaveM wrote recently that they had eliminated
> SPARC's save_flags/restore_flags-in-same-stack-frame requirement.

The breakage that started the discussion regarding 2.6 was a breakage in 
suspend2 (AFAIR on i386).

It's unknown whether there are other breakages in in-kernel code, and I 
strongly prefer three dozen easy compile fixes over possible runtime 
errors.

> >Although fixing it correctly touches at about three dozen files, these 
> >are pretty straightforward patches (removing inlines or moving code 
> >inside files). After all these issues are sorted out in 2.6, the inline 
> >fixes could be backported to 2.4 .
> 
> I'll consider attacking the inlining issues, after I've reviewed my
> cast-as-lvalue and fastcall fixes -- I noticed that at least one fix
> had changed in 2.6 since I first adapted it to 2.4.

I can also attack the inlining issues (I sent most of the 2.6 fixes), 
but I'd prefer to wait until we've fixed all of them in 2.6 before 
backporting the fixes.

> /Mikael

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