Re: [PATCH] Illustration of warning explosion silliness

From: Jeff Garzik
Date: Wed Sep 27 2006 - 21:49:10 EST

Andrew Morton wrote:
On Wed, 27 Sep 2006 20:58:30 -0400
Jeff Garzik <jeff@xxxxxxxxxx> wrote:

The following patch (DO NOT APPLY) illustrates why
device_for_each_child() should not be marked with __must_check.

The function returns the return value of the actor function, and ceases
iteration upon error.

However, _every_ case in drivers/scsi has a hardcoded return value,
illustrating how it is quite valid to not check the return value of this

What does "has a hardcoded return value" mean?

Reference the sentence before that. The return value of the actor passed to device_for_each_child() is always either zero (for some actors) or one (for another actor). In all cases, it is never variable.

AFICT the problem here is that (for example) (going up the call stack in
the callee->caller direction):

scsi_internal_device_block() returns an error code

but device_block() drops that on the floor

so target_block() drops it on the floor too

so scsi_target_block() drops it on the floor too

It's a small matter of (correct kernel) programming to correctly propagate
the scsi_internal_device_block() error code all the way back out of

It all looks rather sloppy?

Quite sloppy. But that doesn't change the fact that device_for_each_child()'s actor _may_ hardcode the return value. It's a valid usage model for that function.

If you are doing a simple collection of data -- adding items to a preallocating list or bitmap -- or doing a search, as with __remove_child() in drivers/scsi/scsi_sysfs.c, the return value can be quite useless.

The usage model should not be _forced_ upon the caller, since it might not be needed.


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at