On Fri, Nov 22, 2013 at 10:10 AM, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote: >>>> The proposal is dma_request_slave_channel only returns errors or valid >>>> pointers, never NULL. >>> >>> OK, so if you make that assumption, I guess it's safe. >> >> I made that assumption because that is what your original patch proposed: >> >> +/** >> + * dma_request_slave_channel_or_err - try to allocate an exclusive >> slave channel >> + * @dev: pointer to client device structure >> + * @name: slave channel name >> + * >> + * Returns pointer to appropriate dma channel on success or an error pointer. >> + */ >> >> What's the benefit of leaking NULL values to callers? If they already >> need to check for err, why force them to check for NULL too? > > "Returns pointer to appropriate dma channel on success or an error > pointer." means that callers only have to check for an ERR value. If the > function returns NULL, then other DMA-related functions must treat that > as a valid channel ID. This is case (a) in my previous email. How can a channel be "valid" and NULL at the same time? Without the guarantee that dma_request_channel always returns a non-null-channel pointer or an error pointer you're forcing clients to use or open-code IS_ERR_OR_NULL. Make the caller's life easier and just turn success or failure like before. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html