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

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Thu, 29 Dec 2022 10:39:31 +0100
Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com> wrote:

> On Wed, 28 Dec 2022 10:07:22 -0500
> Jes Sorensen <jes@trained-monkey.org> wrote:
> 
> > On 12/21/22 06:50, Mariusz Tkaczyk wrote:  
> > > Move duplicated code for both config.c and mdadm.c to new functions.
> > > Add error enum in mdadm.h. Use MD_NAME_MAX instead hardcoded value
> > > in mddev_ident. Use secure functions.
> > > 
> > > In next patch POSIX validation is added.
> > > 
> > > Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>    
> > 
> > Hi Mariusz,
> > 
> > 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 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.
> 

Hi Jes,
Should I drop this enum in v2?

Thanks,
Mariusz