[dpdk-dev] [PATCH v6 02/11] eal: introduce internal wrappers for file operations

[Dmitry Kozlyuk]

On Wed, 3 Jun 2020 08:07:59 -0400
Neil Horman <nhorman at tuxdriver.com> wrote:

[snip]
> > +int
> > +eal_file_create(const char *path)
> > +{
> > +	int ret;
> > +
> > +	ret = open(path, O_CREAT | O_RDWR, 0600);
> > +	if (ret < 0)
> > +		rte_errno = errno;
> > +
> > +	return ret;
> > +}
> > +  
> You don't need this call if you support the oflags option in the open call
> below.

See below.

> > +int
> > +eal_file_open(const char *path, bool writable)
> > +{
> > +	int ret, flags;
> > +
> > +	flags = writable ? O_RDWR : O_RDONLY;
> > +	ret = open(path, flags);
> > +	if (ret < 0)
> > +		rte_errno = errno;
> > +
> > +	return ret;
> > +}
> > +  
> why are you changing this api from the posix file format (with oflags
> specified).  As far as I can see both unix and windows platforms support that

There is a number of caveats, which IMO make this approach better:

1. Filesystem permissions on Windows are complicated. Supporting anything
other than 0600 would add a lot of code, while EAL doesn't really need it.
Microsoft's open() takes not permission bits, but a set of flags.

2. Restricted interface prevents EAL developers from accidentally using
features not supported on all platforms via a seemingly rich API.

3. Microsoft CRT (the one Clang is using) deprecates open() in favor of
_sopen_s() and issues a warning, and we're targeting -Werror. Disabling all
such warnings (_CRT_SECURE_NO_DEPRECATE) doesn't seem right when CRT vendor
encourages using alternatives. This is the primary reason for open()
wrappers in v6.

-- 
Dmitry Kozlyuk