On Fri, Aug 12 2016, Dan Carpenter wrote:

> On Fri, Jul 22, 2016 at 03:57:40PM +0200, Hannes Reinecke wrote:
>> > 
>> > I guess that almost all functions return only a few possible error codes?
>> 
>> Precisely. If we had a way of specifying "the return value is an errno
>> with the possible values '0', '-EIO', and '-EINVAL'" that would be
>> _so_ cool.
>
> I think that's a bad idea.  We should be propagating errors from the
> functions we call.  It should be able to change without breaking.

Should we?  I recently faced a bug caused by a (proposed) change to
btrfs which returned a different error code to the ->fh_to_dentry()
function. That was being propagated up to nfsd, and out on the wire in
the NFSv4 protocol.
Only the new error was invalid for the protocol and the client
(correctly) reported it to user-space rather than handling it
internally.

This happened because not enough thought/documentation had been given to
which error codes were sensibly meaningful.  I changed
exportfs_decode_fh() so that any error other than ENOMEM became ESTALE,
because that is all nfsd can sensibly handle.

I'm sure there are some (many!) paths where error codes should be
propagated transparently, but I don't think we should assume that is
always true.

>
> Smatch does a pretty good job of tracking return values, especially
> if you rebuild the database over and over once a day like I do.

So it is OK to keep a list of valid return values in a database, but not
OK to keep them in the code as documentation, and to alert the
programmer when they make a change so they can declare (and maybe even
document) if it was an intentional change?
Maybe I'm just misunderstanding your point of view.

Thanks,
NeilBrown