Re: [PATCH 2/3] mdadm: refactor ident->name handling

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

 



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



[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux