On 12/29/22 04:39, Mariusz Tkaczyk wrote: Hi Mariusz, Apologies for the slow response on this one. > On Wed, 28 Dec 2022 10:07:22 -0500 > Jes Sorensen <jes@xxxxxxxxxxxxxxxxxx> wrote: >> I appreciate the work to consolidate duplicate code. However, I am not a >> fan of new typedefs, in addition you return status_t codes in functions >> changed to return error_t, which is inconsistent. > > Hi Jes, > Indeed, initially I named it as error_t and I forgot to update that part. > I'm surprised that compiler didn't catch it. Thanks! > > About typedef, I did it same for IMSM already: > https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/tree/super-intel.c#n376 > I can change that but I wanted to define a common solution propagated later to > other mdadm parts. I am really on the fence on this one. I'd very much like to see us get away from the nasty 0/1/2 error codes we currently have all over the place, but I am also vary of reinventing the wheel. I must admit I missed it in super-intel.c >> I would prefer if we move towards standard POSIX error codes instead of >> trying to invent new ones. >> > > The POSIX errors are defined for communication with kernel space and > unfortunately they are not detailed enough. For example "undefined" or > just "general_error" statuses are not available. > https://man7.org/linux/man-pages/man3/errno.3.html > It the approach I proposed we are free to create exact errors we need. > Later we can create a map of error values to string and create dedicated > error print functions. I agree that POSIX codes aren't perfect, however at least for the current errors I see reported in this patch -EINVAL or -E2BIG ought to cover. If you think there are many cases where we cannot map well to POSIX, then I would be OK with it, but I would prefer to go straight to a global error code space rather than one per subsystem. Thoughts? Thanks, Jes