* [PATCH] (2.5.66-mm2) War on warnings
@ 2003-04-01 15:22 Martin J. Bligh
2003-04-01 15:27 ` Jeff Garzik
0 siblings, 1 reply; 8+ messages in thread
From: Martin J. Bligh @ 2003-04-01 15:22 UTC (permalink / raw)
To: Andrew Morton, colpatch; +Cc: linux-kernel
drivers/base/node.c: In function `register_node_type':
drivers/base/node.c:96: warning: suggest parentheses around assignment used as truth value
drivers/base/memblk.c: In function `register_memblk_type':
drivers/base/memblk.c:54: warning: suggest parentheses around assignment used as truth value
Bah.
--- linux-2.5.66-mm2/drivers/base/node.c 2003-04-01 06:40:02.000000000 -0800
+++ 2.5.66-mm2/drivers/base/node.c 2003-04-01 06:37:32.000000000 -0800
@@ -93,7 +93,7 @@ int __init register_node_type(void)
{
int error;
if (!(error = devclass_register(&node_devclass)))
- if (error = driver_register(&node_driver))
+ if ((error = driver_register(&node_driver)))
devclass_unregister(&node_devclass);
return error;
}
--- linux-2.5.66-mm2/drivers/base/memblk.c 2003-04-01 06:40:02.000000000 -0800
+++ 2.5.66-mm2/drivers/base/memblk.c 2003-04-01 06:38:14.000000000 -0800
@@ -51,7 +51,7 @@ int __init register_memblk_type(void)
{
int error;
if (!(error = devclass_register(&memblk_devclass)))
- if (error = driver_register(&memblk_driver))
+ if ((error = driver_register(&memblk_driver)))
devclass_unregister(&memblk_devclass);
return error;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 15:22 [PATCH] (2.5.66-mm2) War on warnings Martin J. Bligh
@ 2003-04-01 15:27 ` Jeff Garzik
2003-04-01 16:13 ` Martin J. Bligh
2003-04-01 16:14 ` Juan Quintela
0 siblings, 2 replies; 8+ messages in thread
From: Jeff Garzik @ 2003-04-01 15:27 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Andrew Morton, colpatch, linux-kernel
On Tue, Apr 01, 2003 at 07:22:37AM -0800, Martin J. Bligh wrote:
> drivers/base/node.c: In function `register_node_type':
> drivers/base/node.c:96: warning: suggest parentheses around assignment used as truth value
> drivers/base/memblk.c: In function `register_memblk_type':
> drivers/base/memblk.c:54: warning: suggest parentheses around assignment used as truth value
>
> Bah.
>
> --- linux-2.5.66-mm2/drivers/base/node.c 2003-04-01 06:40:02.000000000 -0800
> +++ 2.5.66-mm2/drivers/base/node.c 2003-04-01 06:37:32.000000000 -0800
> @@ -93,7 +93,7 @@ int __init register_node_type(void)
> {
> int error;
> if (!(error = devclass_register(&node_devclass)))
> - if (error = driver_register(&node_driver))
> + if ((error = driver_register(&node_driver)))
> devclass_unregister(&node_devclass);
Personally, I feel statements like these are prone to continual error
and confusion. I would prefer to break each test like this out into
separate assignment and test statements. Combining them decreases
readability, while saving a paltry few extra bytes of source code.
Sure, the gcc warning is silly, but the code is a bit obtuse too.
Jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 15:27 ` Jeff Garzik
@ 2003-04-01 16:13 ` Martin J. Bligh
2003-04-01 18:37 ` Matthew Dobson
` (2 more replies)
2003-04-01 16:14 ` Juan Quintela
1 sibling, 3 replies; 8+ messages in thread
From: Martin J. Bligh @ 2003-04-01 16:13 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, colpatch, linux-kernel
>> drivers/base/node.c: In function `register_node_type':
>> drivers/base/node.c:96: warning: suggest parentheses around assignment used as truth value
>> drivers/base/memblk.c: In function `register_memblk_type':
>> drivers/base/memblk.c:54: warning: suggest parentheses around assignment used as truth value
>>
>> Bah.
>>
>> --- linux-2.5.66-mm2/drivers/base/node.c 2003-04-01 06:40:02.000000000 -0800
>> +++ 2.5.66-mm2/drivers/base/node.c 2003-04-01 06:37:32.000000000 -0800
>> @@ -93,7 +93,7 @@ int __init register_node_type(void)
>> {
>> int error;
>> if (!(error = devclass_register(&node_devclass)))
>> - if (error = driver_register(&node_driver))
>> + if ((error = driver_register(&node_driver)))
>> devclass_unregister(&node_devclass);
>
> Personally, I feel statements like these are prone to continual error
> and confusion. I would prefer to break each test like this out into
> separate assignment and test statements. Combining them decreases
> readability, while saving a paltry few extra bytes of source code.
>
> Sure, the gcc warning is silly, but the code is a bit obtuse too.
True, I agree with this in general, and I think Andrew does too, from
previous comments. I was just being lazy ;-) More appropriate patch below.
Compile tested, but not run.
M.
diff -urpN -X /home/fletch/.diff.exclude mm2/drivers/base/memblk.c mm2-warnfix/drivers/base/memblk.c
--- mm2/drivers/base/memblk.c Tue Apr 1 08:05:55 2003
+++ mm2-warnfix/drivers/base/memblk.c Tue Apr 1 08:08:36 2003
@@ -50,9 +50,13 @@ int __init register_memblk(struct memblk
int __init register_memblk_type(void)
{
int error;
- if (!(error = devclass_register(&memblk_devclass)))
- if (error = driver_register(&memblk_driver))
+
+ error = devclass_register(&memblk_devclass);
+ if (!error) {
+ error = driver_register(&memblk_driver);
+ if (error)
devclass_unregister(&memblk_devclass);
+ }
return error;
}
postcore_initcall(register_memblk_type);
diff -urpN -X /home/fletch/.diff.exclude mm2/drivers/base/node.c mm2-warnfix/drivers/base/node.c
--- mm2/drivers/base/node.c Tue Apr 1 08:05:55 2003
+++ mm2-warnfix/drivers/base/node.c Tue Apr 1 08:07:36 2003
@@ -92,9 +92,13 @@ int __init register_node(struct node *no
int __init register_node_type(void)
{
int error;
- if (!(error = devclass_register(&node_devclass)))
- if (error = driver_register(&node_driver))
+
+ error = devclass_register(&node_devclass);
+ if (!error) {
+ error = driver_register(&node_driver);
+ if (error)
devclass_unregister(&node_devclass);
+ }
return error;
}
postcore_initcall(register_node_type);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 15:27 ` Jeff Garzik
2003-04-01 16:13 ` Martin J. Bligh
@ 2003-04-01 16:14 ` Juan Quintela
1 sibling, 0 replies; 8+ messages in thread
From: Juan Quintela @ 2003-04-01 16:14 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Martin J. Bligh, Andrew Morton, colpatch, linux-kernel
>>>>> "jeff" == Jeff Garzik <jgarzik@pobox.com> writes:
jeff> On Tue, Apr 01, 2003 at 07:22:37AM -0800, Martin J. Bligh wrote:
>> drivers/base/node.c: In function `register_node_type':
>> drivers/base/node.c:96: warning: suggest parentheses around assignment used as truth value
>> drivers/base/memblk.c: In function `register_memblk_type':
>> drivers/base/memblk.c:54: warning: suggest parentheses around assignment used as truth value
>>
>> Bah.
>>
>> --- linux-2.5.66-mm2/drivers/base/node.c 2003-04-01 06:40:02.000000000 -0800
>> +++ 2.5.66-mm2/drivers/base/node.c 2003-04-01 06:37:32.000000000 -0800
>> @@ -93,7 +93,7 @@ int __init register_node_type(void)
>> {
>> int error;
>> if (!(error = devclass_register(&node_devclass)))
>> - if (error = driver_register(&node_driver))
>> + if ((error = driver_register(&node_driver)))
>> devclass_unregister(&node_devclass);
jeff> Personally, I feel statements like these are prone to continual error
jeff> and confusion. I would prefer to break each test like this out into
jeff> separate assignment and test statements. Combining them decreases
jeff> readability, while saving a paltry few extra bytes of source code.
jeff> Sure, the gcc warning is silly, but the code is a bit obtuse too.
I think:
- gcc warning is good, normally you want to test, not assignation
- Linus style for that is:
if ((error = driver_register(&node_driver)) != 0)
which sounds more logical. the double parens things is just a bad
hack IMHO.
Later, Juan.
--
In theory, practice and theory are the same, but in practice they
are different -- Larry McVoy
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 16:13 ` Martin J. Bligh
@ 2003-04-01 18:37 ` Matthew Dobson
2003-04-01 19:30 ` Matthew Dobson
2003-04-01 19:32 ` Matthew Dobson
2 siblings, 0 replies; 8+ messages in thread
From: Matthew Dobson @ 2003-04-01 18:37 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Jeff Garzik, Andrew Morton, linux-kernel
Martin J. Bligh wrote:
>>>drivers/base/node.c: In function `register_node_type':
>>>drivers/base/node.c:96: warning: suggest parentheses around assignment used as truth value
>>>drivers/base/memblk.c: In function `register_memblk_type':
>>>drivers/base/memblk.c:54: warning: suggest parentheses around assignment used as truth value
>>>
>>>Bah.
>>>
>>>--- linux-2.5.66-mm2/drivers/base/node.c 2003-04-01 06:40:02.000000000 -0800
>>>+++ 2.5.66-mm2/drivers/base/node.c 2003-04-01 06:37:32.000000000 -0800
>>>@@ -93,7 +93,7 @@ int __init register_node_type(void)
>>> {
>>> int error;
>>> if (!(error = devclass_register(&node_devclass)))
>>>- if (error = driver_register(&node_driver))
>>>+ if ((error = driver_register(&node_driver)))
>>> devclass_unregister(&node_devclass);
>>
>>Personally, I feel statements like these are prone to continual error
>>and confusion. I would prefer to break each test like this out into
>>separate assignment and test statements. Combining them decreases
>>readability, while saving a paltry few extra bytes of source code.
>>
>>Sure, the gcc warning is silly, but the code is a bit obtuse too.
>
>
> True, I agree with this in general, and I think Andrew does too, from
> previous comments. I was just being lazy ;-) More appropriate patch below.
> Compile tested, but not run.
>
> M.
Shouldn't there be an accompanying change to drivers/base/cpu.c? I had
sent a patch that fixed all three, I believe. Did the cpu changes get
dropped?
-Matt
>
> diff -urpN -X /home/fletch/.diff.exclude mm2/drivers/base/memblk.c mm2-warnfix/drivers/base/memblk.c
> --- mm2/drivers/base/memblk.c Tue Apr 1 08:05:55 2003
> +++ mm2-warnfix/drivers/base/memblk.c Tue Apr 1 08:08:36 2003
> @@ -50,9 +50,13 @@ int __init register_memblk(struct memblk
> int __init register_memblk_type(void)
> {
> int error;
> - if (!(error = devclass_register(&memblk_devclass)))
> - if (error = driver_register(&memblk_driver))
> +
> + error = devclass_register(&memblk_devclass);
> + if (!error) {
> + error = driver_register(&memblk_driver);
> + if (error)
> devclass_unregister(&memblk_devclass);
> + }
> return error;
> }
> postcore_initcall(register_memblk_type);
> diff -urpN -X /home/fletch/.diff.exclude mm2/drivers/base/node.c mm2-warnfix/drivers/base/node.c
> --- mm2/drivers/base/node.c Tue Apr 1 08:05:55 2003
> +++ mm2-warnfix/drivers/base/node.c Tue Apr 1 08:07:36 2003
> @@ -92,9 +92,13 @@ int __init register_node(struct node *no
> int __init register_node_type(void)
> {
> int error;
> - if (!(error = devclass_register(&node_devclass)))
> - if (error = driver_register(&node_driver))
> +
> + error = devclass_register(&node_devclass);
> + if (!error) {
> + error = driver_register(&node_driver);
> + if (error)
> devclass_unregister(&node_devclass);
> + }
> return error;
> }
> postcore_initcall(register_node_type);
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 16:13 ` Martin J. Bligh
2003-04-01 18:37 ` Matthew Dobson
@ 2003-04-01 19:30 ` Matthew Dobson
2003-04-01 19:32 ` Matthew Dobson
2 siblings, 0 replies; 8+ messages in thread
From: Matthew Dobson @ 2003-04-01 19:30 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Jeff Garzik, Andrew Morton, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 783 bytes --]
Martin J. Bligh wrote:
>>Personally, I feel statements like these are prone to continual error
>>and confusion. I would prefer to break each test like this out into
>>separate assignment and test statements. Combining them decreases
>>readability, while saving a paltry few extra bytes of source code.
>>
>>Sure, the gcc warning is silly, but the code is a bit obtuse too.
>
>
> True, I agree with this in general, and I think Andrew does too, from
> previous comments. I was just being lazy ;-) More appropriate patch below.
> Compile tested, but not run.
>
> M.
Ok... drivers/base/cpu.c had the double parens, unlike memblkc. &
node.c This patch will make them all more readable and consistent...
Down with warnings!! We must liberate them, and set them free! ;)
-Matt
[-- Attachment #2: sysfs_register_fix.patch --]
[-- Type: text/plain, Size: 1947 bytes --]
diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-mm2/drivers/base/cpu.c linux-2.5.66-mm2+warnfix/drivers/base/cpu.c
--- linux-2.5.66-mm2/drivers/base/cpu.c Tue Apr 1 11:22:06 2003
+++ linux-2.5.66-mm2+warnfix/drivers/base/cpu.c Tue Apr 1 11:26:21 2003
@@ -49,8 +49,12 @@
int __init cpu_dev_init(void)
{
int error;
- if (!(error = devclass_register(&cpu_devclass)))
- if ((error = driver_register(&cpu_driver)))
+
+ error = devclass_register(&cpu_devclass);
+ if (!error) {
+ error = driver_register(&cpu_driver);
+ if (error)
devclass_unregister(&cpu_devclass);
+ }
return error;
}
diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-mm2/drivers/base/memblk.c linux-2.5.66-mm2+warnfix/drivers/base/memblk.c
--- linux-2.5.66-mm2/drivers/base/memblk.c Tue Apr 1 11:22:06 2003
+++ linux-2.5.66-mm2+warnfix/drivers/base/memblk.c Tue Apr 1 11:24:51 2003
@@ -50,9 +50,13 @@
int __init register_memblk_type(void)
{
int error;
- if (!(error = devclass_register(&memblk_devclass)))
- if (error = driver_register(&memblk_driver))
+
+ error = devclass_register(&memblk_devclass);
+ if (!error) {
+ error = driver_register(&memblk_driver);
+ if (error)
devclass_unregister(&memblk_devclass);
+ }
return error;
}
postcore_initcall(register_memblk_type);
diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-mm2/drivers/base/node.c linux-2.5.66-mm2+warnfix/drivers/base/node.c
--- linux-2.5.66-mm2/drivers/base/node.c Tue Apr 1 11:22:06 2003
+++ linux-2.5.66-mm2+warnfix/drivers/base/node.c Tue Apr 1 11:24:51 2003
@@ -92,9 +92,13 @@
int __init register_node_type(void)
{
int error;
- if (!(error = devclass_register(&node_devclass)))
- if (error = driver_register(&node_driver))
+
+ error = devclass_register(&node_devclass);
+ if (!error) {
+ error = driver_register(&node_driver);
+ if (error)
devclass_unregister(&node_devclass);
+ }
return error;
}
postcore_initcall(register_node_type);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 16:13 ` Martin J. Bligh
2003-04-01 18:37 ` Matthew Dobson
2003-04-01 19:30 ` Matthew Dobson
@ 2003-04-01 19:32 ` Matthew Dobson
2003-04-01 19:36 ` Martin J. Bligh
2 siblings, 1 reply; 8+ messages in thread
From: Matthew Dobson @ 2003-04-01 19:32 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Jeff Garzik, Andrew Morton, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 104 bytes --]
And whilst we're stomping warnings, here are a couple more I noticed in
various compilations...
-Matt
[-- Attachment #2: warning_fixes-2.5.66.patch --]
[-- Type: text/plain, Size: 1799 bytes --]
diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-vanilla/drivers/char/drm/r128_cce.c linux-2.5.66-warnings/drivers/char/drm/r128_cce.c
--- linux-2.5.66-vanilla/drivers/char/drm/r128_cce.c Mon Mar 24 14:00:07 2003
+++ linux-2.5.66-warnings/drivers/char/drm/r128_cce.c Mon Mar 31 11:55:16 2003
@@ -352,7 +352,7 @@
entry->busaddr[page_ofs]);
DRM_DEBUG( "ring rptr: offset=0x%08x handle=0x%08lx\n",
entry->busaddr[page_ofs],
- entry->handle + tmp_ofs );
+ (unsigned long)entry->handle + tmp_ofs );
}
/* Set watermark control */
diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-vanilla/drivers/net/tulip/interrupt.c linux-2.5.66-warnings/drivers/net/tulip/interrupt.c
--- linux-2.5.66-vanilla/drivers/net/tulip/interrupt.c Mon Mar 24 14:00:10 2003
+++ linux-2.5.66-warnings/drivers/net/tulip/interrupt.c Mon Mar 31 11:55:16 2003
@@ -198,7 +198,7 @@
dev->name,
le32_to_cpu(tp->rx_ring[entry].buffer1),
tp->rx_buffers[entry].mapping,
- skb->head, temp);
+ (unsigned long)skb->head, temp);
}
#endif
diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-vanilla/drivers/scsi/scsi_sysfs.c linux-2.5.66-warnings/drivers/scsi/scsi_sysfs.c
--- linux-2.5.66-vanilla/drivers/scsi/scsi_sysfs.c Mon Mar 24 14:00:08 2003
+++ linux-2.5.66-warnings/drivers/scsi/scsi_sysfs.c Mon Mar 31 11:56:02 2003
@@ -272,14 +272,17 @@
return 0;
}
+void scsi_rescan_device(struct scsi_device *);
static ssize_t
store_rescan_field (struct device *dev, const char *buf, size_t count)
{
int ret = ENODEV;
struct scsi_device *sdev;
sdev = to_scsi_device(dev);
- if (sdev)
- ret = scsi_rescan_device(sdev);
+ if (sdev){
+ ret = 0;
+ scsi_rescan_device(sdev);
+ }
return ret;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] (2.5.66-mm2) War on warnings
2003-04-01 19:32 ` Matthew Dobson
@ 2003-04-01 19:36 ` Martin J. Bligh
0 siblings, 0 replies; 8+ messages in thread
From: Martin J. Bligh @ 2003-04-01 19:36 UTC (permalink / raw)
To: colpatch; +Cc: Jeff Garzik, Andrew Morton, linux-kernel
> diff -Nur --exclude-from=/usr/src/.dontdiff linux-2.5.66-vanilla/drivers/char/drm/r128_cce.c linux-2.5.66-warnings/drivers/char/drm/r128_cce.c
> --- linux-2.5.66-vanilla/drivers/char/drm/r128_cce.c Mon Mar 24 14:00:07 2003
> +++ linux-2.5.66-warnings/drivers/char/drm/r128_cce.c Mon Mar 31 11:55:16 2003
> @@ -352,7 +352,7 @@
> entry->busaddr[page_ofs]);
> DRM_DEBUG( "ring rptr: offset=0x%08x handle=0x%08lx\n",
> entry->busaddr[page_ofs],
> - entry->handle + tmp_ofs );
> + (unsigned long)entry->handle + tmp_ofs );
> }
>
> /* Set watermark control */
These sort of things really need to be typecast to u64 if that's
the dma_addr_t printk problem ... otherwise you silently lose data,
which is most confusing.
linux-2.5.66-vanilla/drivers/scsi/scsi_sysfs.c linux-2.5.66-warnings/drivers/scsi/scsi_sysfs.c
> --- linux-2.5.66-vanilla/drivers/scsi/scsi_sysfs.c Mon Mar 24 14:00:08 2003
> +++ linux-2.5.66-warnings/drivers/scsi/scsi_sysfs.c Mon Mar 31 11:56:02 2003
> @@ -272,14 +272,17 @@
> return 0;
> }
>
> +void scsi_rescan_device(struct scsi_device *);
> static ssize_t
> store_rescan_field (struct device *dev, const char *buf, size_t count)
> {
> int ret = ENODEV;
> struct scsi_device *sdev;
> sdev = to_scsi_device(dev);
> - if (sdev)
> - ret = scsi_rescan_device(sdev);
> + if (sdev){
> + ret = 0;
> + scsi_rescan_device(sdev);
> + }
> return ret;
> }
That's pretty much what I did, but apparently Christoph had a better fix
posted to linux-scsi somewhere. I lost it though ...
M.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2003-04-01 19:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-04-01 15:22 [PATCH] (2.5.66-mm2) War on warnings Martin J. Bligh
2003-04-01 15:27 ` Jeff Garzik
2003-04-01 16:13 ` Martin J. Bligh
2003-04-01 18:37 ` Matthew Dobson
2003-04-01 19:30 ` Matthew Dobson
2003-04-01 19:32 ` Matthew Dobson
2003-04-01 19:36 ` Martin J. Bligh
2003-04-01 16:14 ` Juan Quintela
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).