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

From: Ratheesh Kannoth <rkannoth@marvell.com>
To: Max Gautier <mg@max.gautier.name>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH iproute2-next v2] arpd: create /var/lib/arpd on first use
Date: Mon, 18 Mar 2024 08:51:40 +0000	[thread overview]
Message-ID: <MWHPR1801MB1918B6880C90E045C219B9ADD32D2@MWHPR1801MB1918.namprd18.prod.outlook.com> (raw)
In-Reply-To: <Zff9ReznTN4h-Jrh@framework>

> 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. 

> 
> > > +			) {
> > > +		perror("create_db_dir");
> > > +		exit(-1);
> > > +	}
> > > +
> > >  	dbase = dbopen(dbname, O_CREAT|O_RDWR, 0644, DB_HASH,
> NULL);
> > >  	if (dbase == NULL) {
> > >  		perror("db_open");
> > > --
> > > 2.44.0
> > >
> 
> --
> Max Gautier