On Wed, Mar 19, 2014 at 09:57:26PM +0100, Hannes Reinecke wrote:
> Before I attempt this the codebase needs some in-depth cleanup.
> Main problem here is the inconsistent usage of -1, 0, and 1.
> The ->probefunc functions use 0 to signal 'not found',
I see that ->probefunc functions (FS or partition specific functions)
use <0 = errors, 1 = not found and 0 = success.
> partitions_probe uses the inverse, namely '0' signals found.
>
> And so on and so forth.
>
> Plus there are even some vestiges of an attempted error code
> handling; BLKID_ERR_* are used occasionally.
>
> It would be good if one could agree on a common strategy here,
The library has too two parts,
1/ original high-level (based on blkid_cache)
2/ low-level (based on blkid_probe, probe.c, partitions/ and
superblocks/)
My suggestion is to care about the low-level part for now.
The low-level part has three layers:
a) API (blkid_do_probe(), blkid_do_safeprobe(), ...)
now:
1 nothing, 0 success, <0 error, -2 ambivalent result
suggestion:
1 nothing, 0 success, -1 error, -2 ambivalent result
+ modify errno only if we return -1 and original errno is still
0 (it means that error has been generated by our library rather
rather than a syscall).
Note that this is part of API, so we have to very careful.
b) chain driver (struct blkid_chaindrv, superblock.c and
partitions.c)
now:
1 nothing, 0 success, <0 error, -2 ambivalent
my suggestion:
2 ambivalent, 1 nothing, 0 success, <0 error
In a) and b) is small collision between "ambivalent" and "error"
return codes. That's mistake that should be fixed.
c) FS/partition specific probing functions:
now:
..mess...
my suggestion:
0 success, <0 error, 1 nothing
where for b) and c) we can return real error codes (e.g. -errno)
rather than too generic -1. My suggestion is not to modify errno
there. If we want to touch errno then in the top-level library
functions (layer a) only. In the b) and c) We need things like
return -EINVAL;
without care about errno.
> and audit the code base to adhere to this.
> Once that is done we can work on returning correct error codes.
I think we don't need two iterations.
> What do you prefer?
0) make a decision about the return codes
1) add to the code (blkidP.h ?) hint about the codes
2) clean up the code
- start from the layer c) then b) and a)
- from the beginning use real error codes rather than
the generic -1, if necessary use -EINVAL, -ENOMEM, etc.
- don't rely on errno to return error codes from
probing functions.
(BTW, I'm on #util-linux on freenode IRC if you want to talk about it
more. Maybe I can help you and cleanup some part of the library too,
it's my mess ;-))
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
