drivers/base: Remove unnecessary OOM message

This patch fix checkpatch warnings concerning the possible duplication of an "out of memory" message.

A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)

@@
identifier f,print,l;
expression e;
constant char[] c;
@@

e = \(kzalloc\|kmalloc\|devm_kzalloc\|devm_kmalloc\)(...); if (e == NULL) {
<+...
- print(...,c,...);
... when any
(
goto l;
|
return ...;
)
...+> }

---
 drivers/base/firmware_class.c  | 1 -
 drivers/base/power/clock_ops.c | 4 +---
 drivers/base/power/opp.c       | 8 ++------
 3 files changed, 3 insertions(+), 10 deletions(-)

index 58470c3..c3293f0 100644
@@ -855,7 +855,6 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 
 	fw_priv = kzalloc(sizeof(*fw_priv), GFP_KERNEL);
 	if (!fw_priv) {
-		dev_err(device, "%s: kmalloc failed\n", __func__);
 		fw_priv = ERR_PTR(-ENOMEM);
 		goto exit;
 	}

index d626576..7fdd017 100644
@@ -81,10 +81,8 @@ static int __pm_clk_add(struct device *dev, const char *con_id,
 		return -EINVAL;
 
 	ce = kzalloc(sizeof(*ce), GFP_KERNEL);
-	if (!ce) {
-		dev_err(dev, "Not enough memory for clock entry.\n");
+	if (!ce)
 		return -ENOMEM;
-	}
 
 	if (con_id) {
 		ce->con_id = kstrdup(con_id, GFP_KERNEL);

index 15bf299..677fb28 100644
@@ -474,10 +474,8 @@ static int _opp_add_dynamic(struct device *dev, unsigned long freq,
 
 	/* allocate new OPP node */
 	new_opp = kzalloc(sizeof(*new_opp), GFP_KERNEL);
-	if (!new_opp) {
-		dev_warn(dev, "%s: Unable to create new OPP node\n", __func__);
+	if (!new_opp)
 		return -ENOMEM;
-	}
 
 	/* Hold our list modification lock here */
 	mutex_lock(&dev_opp_list_lock);

@@ -695,10 +693,8 @@ static int _opp_set_availability(struct device *dev, unsigned long freq,
 
 	/* keep the node allocated */
 	new_opp = kmalloc(sizeof(*new_opp), GFP_KERNEL);
-	if (!new_opp) {
-		dev_warn(dev, "%s: Unable to create OPP\n", __func__);
+	if (!new_opp)
 		return -ENOMEM;
-	}
 
 	mutex_lock(&dev_opp_list_lock);
 

1.9.1

So, instead of nice and readable "not enough memory for clock..." we get OOM, stackdump, and backtrace...? Not sure it is improvement.

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

All allocs without __GFP_NOWARN already gets an OOM and stackdump. These are just unnecessary.

"These" being human readable messages or ugly stackdumps?

Read what I have said. Figuring out what allocation failed from unreliable backtrace is not fun.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Facts:
1) None of these four error messages will ever be printed.
http://lwn.net/Articles/627419/
2) These messages are a waste of RAM.

Debatable:
1) These messages are bad style and distracting. Simple code is better.
2) The normal OOM message is enough to find which allocation failed.

Not everbody, but a lot of people write these kinds of error messages with their brain on auto-pilot because they think they *should* do it. It's quite common to do things like printk("dev is NULL. %s", dev->name);. I'm happy for this checkpatch warning because it hopefully bumps people out of mindless mode and makes them think about error messages.

regards,
dan carpenter

So this is what the changelog should be saying, not that it "fixes checkpatch warnings".

checkpatch warnings in *existing* code are meaningless, unless they uncover a *real* problem in it which quite arguably this one is. But in those cases patch changelogs should talk about the *real* problems and they don't even need to mention checkpatch then as far as I'm concerned.

Send this one with a better changelog and I'll apply it.

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.