Re: [PATCH] Illustration of warning explosion silliness

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andrew Morton wrote:
On Wed, 27 Sep 2006 20:58:30 -0400
Jeff Garzik <[email protected]> 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
function.


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
scsi_target_block().

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.

	Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux