RE: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use

[Ratheesh Kannoth <rkannoth@marvell.com>]

> From: Max Gautier <mg@max.gautier.name>
> Sent: Monday, March 18, 2024 2:29 PM
> To: Ratheesh Kannoth <rkannoth@marvell.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create
> /var/lib/arpd on first use
> 
> On Mon, Mar 18, 2024 at 08:51:40AM +0000, Ratheesh Kannoth wrote:
> > > From: Max Gautier <mg@max.gautier.name>
> > > Sent: Monday, March 18, 2024 2:07 PM
> > > To: Ratheesh Kannoth <rkannoth@marvell.com>
> > > Cc: netdev@vger.kernel.org
> > > Subject: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create
> > > /var/lib/arpd on first use
> >
> > > > > +	if (strcmp(default_dbname, dbname) == 0
> > > > > +			&& mkdir(ARPDDIR, 0755) != 0
> > > > > +			&& errno != EEXIST
> > > > why do you need errno != EEXIST case ? mkdir() will return error
> > > > in this case
> > > as well.
> > >
> > > EEXIST is not an error in this case: if the default location already
> > > exist, all is good. mkdir would still return -1 in this case, so we
> > > need to exclude it manually.
> >
> > ACK. IMO, it would make a more readable code if you consider splitting the
> "if" loop.
> 
> Something like this ? I tend to pack conditions unless branching is necessary,
> but no problem if this form is preferred.
> 
> if (strcmp(default_dbname, dbname) == 0) {
>     if (mkdir(ARPDDIR, 0755) != 0 && errno != EEXIST) {
>    ...
>    }
> }
ACK.   
instead of errno != EXIST ,  you may consider stat() before mkdir() call. Just my way thinking(please ignore it, if you don't like). 
My thinking is --> you need to execute mkdir () only first time, second time onwards, stat() call will return 0.