Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
Em 25-01-2011 20:54, Peter Hüwe escreveu:
Am Dienstag 25 Januar 2011, 23:20:44 schrieb Julia Lawall:
On Tue, 25 Jan 2011, Peter Huewe wrote:
This patch fixes the warning Using plain integer as NULL pointer,
generated by sparse, by replacing the offending 0s with NULL.
I recall (a number of years ago) being told that for things like kmalloc,
the proper test was !x, not x == NULL.
julia
Hi Julia,
thanks for your input.
So do I understand you correctly if I say
if(!x) is better than if(x==NULL) in any case?
Or only for the kmalloc family?
Do you remember the reason why !x should be preferred?
In Documentation/CodingStyle , Chapter 7: Centralized exiting of functions
there is a function fun with looks like this:
int fun(int a)
{
int result = 0;
char *buffer = kmalloc(SIZE);
if (buffer == NULL)
return -ENOMEM;
if (condition1) {
while (loop1) {
...
}
result = 1;
goto out;
}
...
out:
kfree(buffer);
return result;
}
-- So if (buffer == NULL) is in the official CodingStyle - maybe we
should
add a paragraph there as well ;)
Don't get me wrong, I just want to learn ;)
Both ways are acceptable. But because C is a Spartan language, and because
I need to review lots of code, I prefer the more synthetic way:
if (!buf)
That means less things to read, and saves me a few microsseconds of reading
and processing it on my mind. In general, such tests occur just after a malloc
or a malloc-like function, so it is really obvious that you're testing for
a pointer.
Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
This patch fixes the warning Using plain integer as NULL pointer,
generated by sparse, by replacing the offending 0s with NULL.
KernelVersion: linus' tree-c723fdab
Signed-off-by: Peter Huewe peterhu...@gmx.de
---
drivers/media/video/saa7164/saa7164-api.c |8
drivers/media/video/saa7164/saa7164-buffer.c | 14 +++---
drivers/media/video/saa7164/saa7164-bus.c |8
drivers/media/video/saa7164/saa7164-cmd.c | 10 +-
drivers/media/video/saa7164/saa7164-core.c|8
drivers/media/video/saa7164/saa7164-dvb.c |4 ++--
drivers/media/video/saa7164/saa7164-encoder.c |6 +++---
drivers/media/video/saa7164/saa7164-fw.c |2 +-
drivers/media/video/saa7164/saa7164-vbi.c |6 +++---
9 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/media/video/saa7164/saa7164-api.c
b/drivers/media/video/saa7164/saa7164-api.c
index bd86d97..c49dcad 100644
--- a/drivers/media/video/saa7164/saa7164-api.c
+++ b/drivers/media/video/saa7164/saa7164-api.c
@@ -743,7 +743,7 @@ int saa7164_api_configure_dif(struct saa7164_port *port,
u32 std)
int saa7164_api_initialize_dif(struct saa7164_port *port)
{
struct saa7164_dev *dev = port-dev;
- struct saa7164_port *p = 0;
+ struct saa7164_port *p = NULL;
int ret = -EINVAL;
u32 std = 0;
@@ -926,9 +926,9 @@ int saa7164_api_configure_port_mpeg2ps(struct saa7164_dev
*dev,
int saa7164_api_dump_subdevs(struct saa7164_dev *dev, u8 *buf, int len)
{
- struct saa7164_port *tsport = 0;
- struct saa7164_port *encport = 0;
- struct saa7164_port *vbiport = 0;
+ struct saa7164_port *tsport = NULL;
+ struct saa7164_port *encport = NULL;
+ struct saa7164_port *vbiport = NULL;
u32 idx, next_offset;
int i;
struct tmComResDescrHeader *hdr, *t;
diff --git a/drivers/media/video/saa7164/saa7164-buffer.c
b/drivers/media/video/saa7164/saa7164-buffer.c
index ddd2521..977062f 100644
--- a/drivers/media/video/saa7164/saa7164-buffer.c
+++ b/drivers/media/video/saa7164/saa7164-buffer.c
@@ -93,7 +93,7 @@ struct saa7164_buffer *saa7164_buffer_alloc(struct
saa7164_port *port,
u32 len)
{
struct tmHWStreamParameters *params = port-hw_streamingparams;
- struct saa7164_buffer *buf = 0;
+ struct saa7164_buffer *buf = NULL;
struct saa7164_dev *dev = port-dev;
int i;
@@ -157,7 +157,7 @@ fail2:
fail1:
kfree(buf);
- buf = 0;
+ buf = NULL;
ret:
return buf;
}
@@ -289,14 +289,14 @@ struct saa7164_user_buffer
*saa7164_buffer_alloc_user(struct saa7164_dev *dev,
struct saa7164_user_buffer *buf;
buf = kzalloc(sizeof(struct saa7164_user_buffer), GFP_KERNEL);
- if (buf == 0)
- return 0;
+ if (buf == NULL)
+ return NULL;
buf-data = kzalloc(len, GFP_KERNEL);
- if (buf-data == 0) {
+ if (buf-data == NULL) {
kfree(buf);
- return 0;
+ return NULL;
}
buf-actual_size = len;
@@ -315,7 +315,7 @@ void saa7164_buffer_dealloc_user(struct saa7164_user_buffer
*buf)
return;
kfree(buf-data);
- buf-data = 0;
+ buf-data = NULL;
kfree(buf);
}
diff --git a/drivers/media/video/saa7164/saa7164-bus.c
b/drivers/media/video/saa7164/saa7164-bus.c
index b2b0d97..466e1b0 100644
--- a/drivers/media/video/saa7164/saa7164-bus.c
+++ b/drivers/media/video/saa7164/saa7164-bus.c
@@ -158,7 +158,7 @@ int saa7164_bus_set(struct saa7164_dev *dev, struct
tmComResInfo* msg,
return SAA_ERR_BAD_PARAMETER;
}
- if ((msg-size 0) (buf == 0)) {
+ if ((msg-size 0) (buf == NULL)) {
printk(KERN_ERR %s() Missing message buffer\n, __func__);
return SAA_ERR_BAD_PARAMETER;
}
@@ -315,7 +315,7 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct
tmComResInfo* msg,
saa7164_bus_verify(dev);
- if (msg == 0)
+ if (msg == NULL)
return ret;
if (msg-size dev-bus.m_wMaxReqSize) {
@@ -324,7 +324,7 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct
tmComResInfo* msg,
return ret;
}
- if ((peekonly == 0) (msg-size 0) (buf == 0)) {
+ if ((peekonly == 0) (msg-size 0) (buf == NULL)) {
printk(KERN_ERR
%s() Missing msg buf, size should be %d bytes\n,
__func__, msg-size);
@@ -392,7 +392,7 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct
tmComResInfo* msg,
printk(KERN_ERR %s() Unexpected msg miss-match\n, __func__);
saa7164_bus_dumpmsg(dev, msg, buf);
- saa7164_bus_dumpmsg(dev, msg_tmp, 0);
+ saa7164_bus_dumpmsg(dev, msg_tmp, NULL);
ret = SAA_ERR_INVALID_COMMAND;
goto out;
}
Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
On Tue, 25 Jan 2011, Peter Huewe wrote:
This patch fixes the warning Using plain integer as NULL pointer,
generated by sparse, by replacing the offending 0s with NULL.
I recall (a number of years ago) being told that for things like kmalloc,
the proper test was !x, not x == NULL.
julia
KernelVersion: linus' tree-c723fdab
Signed-off-by: Peter Huewe peterhu...@gmx.de
---
drivers/media/video/saa7164/saa7164-api.c |8
drivers/media/video/saa7164/saa7164-buffer.c | 14 +++---
drivers/media/video/saa7164/saa7164-bus.c |8
drivers/media/video/saa7164/saa7164-cmd.c | 10 +-
drivers/media/video/saa7164/saa7164-core.c|8
drivers/media/video/saa7164/saa7164-dvb.c |4 ++--
drivers/media/video/saa7164/saa7164-encoder.c |6 +++---
drivers/media/video/saa7164/saa7164-fw.c |2 +-
drivers/media/video/saa7164/saa7164-vbi.c |6 +++---
9 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/media/video/saa7164/saa7164-api.c
b/drivers/media/video/saa7164/saa7164-api.c
index bd86d97..c49dcad 100644
--- a/drivers/media/video/saa7164/saa7164-api.c
+++ b/drivers/media/video/saa7164/saa7164-api.c
@@ -743,7 +743,7 @@ int saa7164_api_configure_dif(struct saa7164_port *port,
u32 std)
int saa7164_api_initialize_dif(struct saa7164_port *port)
{
struct saa7164_dev *dev = port-dev;
- struct saa7164_port *p = 0;
+ struct saa7164_port *p = NULL;
int ret = -EINVAL;
u32 std = 0;
@@ -926,9 +926,9 @@ int saa7164_api_configure_port_mpeg2ps(struct saa7164_dev
*dev,
int saa7164_api_dump_subdevs(struct saa7164_dev *dev, u8 *buf, int len)
{
- struct saa7164_port *tsport = 0;
- struct saa7164_port *encport = 0;
- struct saa7164_port *vbiport = 0;
+ struct saa7164_port *tsport = NULL;
+ struct saa7164_port *encport = NULL;
+ struct saa7164_port *vbiport = NULL;
u32 idx, next_offset;
int i;
struct tmComResDescrHeader *hdr, *t;
diff --git a/drivers/media/video/saa7164/saa7164-buffer.c
b/drivers/media/video/saa7164/saa7164-buffer.c
index ddd2521..977062f 100644
--- a/drivers/media/video/saa7164/saa7164-buffer.c
+++ b/drivers/media/video/saa7164/saa7164-buffer.c
@@ -93,7 +93,7 @@ struct saa7164_buffer *saa7164_buffer_alloc(struct
saa7164_port *port,
u32 len)
{
struct tmHWStreamParameters *params = port-hw_streamingparams;
- struct saa7164_buffer *buf = 0;
+ struct saa7164_buffer *buf = NULL;
struct saa7164_dev *dev = port-dev;
int i;
@@ -157,7 +157,7 @@ fail2:
fail1:
kfree(buf);
- buf = 0;
+ buf = NULL;
ret:
return buf;
}
@@ -289,14 +289,14 @@ struct saa7164_user_buffer
*saa7164_buffer_alloc_user(struct saa7164_dev *dev,
struct saa7164_user_buffer *buf;
buf = kzalloc(sizeof(struct saa7164_user_buffer), GFP_KERNEL);
- if (buf == 0)
- return 0;
+ if (buf == NULL)
+ return NULL;
buf-data = kzalloc(len, GFP_KERNEL);
- if (buf-data == 0) {
+ if (buf-data == NULL) {
kfree(buf);
- return 0;
+ return NULL;
}
buf-actual_size = len;
@@ -315,7 +315,7 @@ void saa7164_buffer_dealloc_user(struct
saa7164_user_buffer *buf)
return;
kfree(buf-data);
- buf-data = 0;
+ buf-data = NULL;
kfree(buf);
}
diff --git a/drivers/media/video/saa7164/saa7164-bus.c
b/drivers/media/video/saa7164/saa7164-bus.c
index b2b0d97..466e1b0 100644
--- a/drivers/media/video/saa7164/saa7164-bus.c
+++ b/drivers/media/video/saa7164/saa7164-bus.c
@@ -158,7 +158,7 @@ int saa7164_bus_set(struct saa7164_dev *dev, struct
tmComResInfo* msg,
return SAA_ERR_BAD_PARAMETER;
}
- if ((msg-size 0) (buf == 0)) {
+ if ((msg-size 0) (buf == NULL)) {
printk(KERN_ERR %s() Missing message buffer\n, __func__);
return SAA_ERR_BAD_PARAMETER;
}
@@ -315,7 +315,7 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct
tmComResInfo* msg,
saa7164_bus_verify(dev);
- if (msg == 0)
+ if (msg == NULL)
return ret;
if (msg-size dev-bus.m_wMaxReqSize) {
@@ -324,7 +324,7 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct
tmComResInfo* msg,
return ret;
}
- if ((peekonly == 0) (msg-size 0) (buf == 0)) {
+ if ((peekonly == 0) (msg-size 0) (buf == NULL)) {
printk(KERN_ERR
%s() Missing msg buf, size should be %d bytes\n,
__func__, msg-size);
@@ -392,7 +392,7 @@ int saa7164_bus_get(struct saa7164_dev *dev, struct
tmComResInfo* msg,
printk(KERN_ERR %s() Unexpected msg miss-match\n, __func__);
saa7164_bus_dumpmsg(dev, msg, buf);
-
Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
Am Dienstag 25 Januar 2011, 23:20:44 schrieb Julia Lawall:
On Tue, 25 Jan 2011, Peter Huewe wrote:
This patch fixes the warning Using plain integer as NULL pointer,
generated by sparse, by replacing the offending 0s with NULL.
I recall (a number of years ago) being told that for things like kmalloc,
the proper test was !x, not x == NULL.
julia
Hi Julia,
thanks for your input.
So do I understand you correctly if I say
if(!x) is better than if(x==NULL) in any case?
Or only for the kmalloc family?
Do you remember the reason why !x should be preferred?
In Documentation/CodingStyle , Chapter 7: Centralized exiting of functions
there is a function fun with looks like this:
int fun(int a)
{
int result = 0;
char *buffer = kmalloc(SIZE);
if (buffer == NULL)
return -ENOMEM;
if (condition1) {
while (loop1) {
...
}
result = 1;
goto out;
}
...
out:
kfree(buffer);
return result;
}
-- So if (buffer == NULL) is in the official CodingStyle - maybe we should
add a paragraph there as well ;)
Don't get me wrong, I just want to learn ;)
Thanks,
Peter
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
On Tue, Jan 25, 2011 at 5:54 PM, Peter Hüwe peterhu...@gmx.de wrote:
Hi Julia,
thanks for your input.
So do I understand you correctly if I say
if(!x) is better than if(x==NULL) in any case?
Or only for the kmalloc family?
Do you remember the reason why !x should be preferred?
In Documentation/CodingStyle , Chapter 7: Centralized exiting of functions
there is a function fun with looks like this:
int fun(int a)
{
int result = 0;
char *buffer = kmalloc(SIZE);
if (buffer == NULL)
return -ENOMEM;
if (condition1) {
while (loop1) {
...
}
result = 1;
goto out;
}
...
out:
kfree(buffer);
return result;
}
-- So if (buffer == NULL) is in the official CodingStyle - maybe we should
add a paragraph there as well ;)
Don't get me wrong, I just want to learn ;)
To my knowledge, the current CodingStyle doesn't enforce a particular
standard in this regard, leaving it at the discretion of the author.
Whether to do (!foo) or (foo == NULL) is one of those debates people
have similar to whether to use tabs as whitespace. People have
differing opinions and there is no clearly right answer. Personally
I strongly prefer (foo == NULL) as it makes it blindingly obvious that
it's a pointer comparison, whereas (!foo) leaves you wondering whether
it's an integer or pointer comparison.
All that said, you shouldn't submit patches which arbitrarily change
from one format to the other. With regards to the proposed patch, you
should follow whatever style the author employed in the rest of the
file.
Cheers,
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
On Tue, 2011-01-25 at 18:05 -0500, Devin Heitmueller wrote:
On Tue, Jan 25, 2011 at 5:54 PM, Peter Hüwe peterhu...@gmx.de wrote:
Hi Julia,
thanks for your input.
So do I understand you correctly if I say
if(!x) is better than if(x==NULL) in any case?
The machine code should be equivalent in size and speed.
Or only for the kmalloc family?
Do you remember the reason why !x should be preferred?
In Documentation/CodingStyle , Chapter 7: Centralized exiting of functions
there is a function fun with looks like this:
int fun(int a)
{
int result = 0;
char *buffer = kmalloc(SIZE);
if (buffer == NULL)
return -ENOMEM;
-- So if (buffer == NULL) is in the official CodingStyle - maybe we
should
add a paragraph there as well ;)
CodingStyle shouldn't specify anything on the matter. There is no
overall, optimal choice for all contexts. Arguing either way is as
pointless as the Lilliputians' little-end vs. big-end dispute.
To my knowledge, the current CodingStyle doesn't enforce a particular
standard in this regard, leaving it at the discretion of the author.
Correct, it does not. I just checked CodingStyle and checkpatch
yesterday.
Whether to do (!foo) or (foo == NULL) is one of those debates people
have similar to whether to use tabs as whitespace. People have
differing opinions and there is no clearly right answer.
It depends on one's measurement criteria for optimizing the written
form of source code.
I prefer more explicit statement of action is taking place over
statements with fewer characters. It usually saves me time when
revisiting code.
More genrally I prefer any coding practice that saves me time when
revisiting code. (Note the word me carries a lot of context with it.)
Ambiguity and implicit behaviors ultimately waste my time.
Personally
I strongly prefer (foo == NULL) as it makes it blindingly obvious that
it's a pointer comparison, whereas (!foo) leaves you wondering whether
it's an integer or pointer comparison.
usenet
Me too.
/usenet
All that said, you shouldn't submit patches which arbitrarily change
from one format to the other. With regards to the proposed patch, you
should follow whatever style the author employed in the rest of the
file.
That is another reasonable critereon in optimizing the written form of
the source code. I tend to give it less weight though.
Regards,
Andy
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] video/saa7164: Fix sparse warning: Using plain integer as NULL pointer
On Tue, 25 Jan 2011, Peter Hüwe wrote:
Am Dienstag 25 Januar 2011, 23:20:44 schrieb Julia Lawall:
On Tue, 25 Jan 2011, Peter Huewe wrote:
This patch fixes the warning Using plain integer as NULL pointer,
generated by sparse, by replacing the offending 0s with NULL.
I recall (a number of years ago) being told that for things like kmalloc,
the proper test was !x, not x == NULL.
julia
Hi Julia,
thanks for your input.
So do I understand you correctly if I say
if(!x) is better than if(x==NULL) in any case?
No.
Or only for the kmalloc family?
Do you remember the reason why !x should be preferred?
Because it is a function call, and NULL represents failure of that
function, not an actual NULL value.
Here is an email that explains that:
http://lkml.org/lkml/2007/7/27/103
Here is the beginning of the thread:
http://lkml.org/lkml/2007/7/27/75
julia
In Documentation/CodingStyle , Chapter 7: Centralized exiting of functions
there is a function fun with looks like this:
int fun(int a)
{
int result = 0;
char *buffer = kmalloc(SIZE);
if (buffer == NULL)
return -ENOMEM;
if (condition1) {
while (loop1) {
...
}
result = 1;
goto out;
}
...
out:
kfree(buffer);
return result;
}
-- So if (buffer == NULL) is in the official CodingStyle - maybe we
should
add a paragraph there as well ;)
Don't get me wrong, I just want to learn ;)
Thanks,
Peter
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
