On Wed, Aug 26, 2020 at 01:19:53PM +0200, Markus Armbruster wrote:
> Now back to my dislike of the #ifdeffery I voiced in reply to PATCH 2.
>
> Code now:
>
> #ifdef O_CLOEXEC
> flags |= O_CLOEXEC;
> #endif /* O_CLOEXEC */
>
> ret = open(name, flags, mode);
>
> #ifndef O_CLOEXEC
> if (ret >= 0) {
> qemu_set_cloexec(ret);
> }
> #endif /* ! O_CLOEXEC */
>
> if (ret == -1) {
> const char *action = flags & O_CREAT ? "create" : "open";
> #ifdef O_DIRECT
> if (errno == EINVAL && (flags & O_DIRECT)) {
> ret = open(name, flags & ~O_DIRECT, mode);
> if (ret != -1) {
> close(ret);
> [O_DIRECT not supported error...]
> errno = EINVAL; /* close() clobbered earlier errno */
> return -1;
> }
> }
> #endif /* O_DIRECT */
> [generic error...]
> }
>
> Compare:
>
> #ifdef O_CLOEXEC
> flags |= O_CLOEXEC;
> ret = open(name, flags, mode);
> #else
> ret = open(name, flags, mode);
> if (ret >= 0) {
> qemu_set_cloexec(ret);
> }
> #endif /* ! O_CLOEXEC */
>
> if (ret == -1) {
> const char *action = flags & O_CREAT ? "create" : "open";
> #ifdef O_DIRECT
> if (errno == EINVAL && (flags & O_DIRECT)) {
> ret = open(name, flags & ~O_DIRECT, mode);
> if (ret != -1) {
> close(ret);
> [O_DIRECT not supported error...]
> errno = EINVAL; /* close() clobbered earlier errno */
> return -1;
> }
> }
> #endif /* O_DIRECT */
> [generic error...]
> }
>
> I like this a bit better, but not enough to make a strong
> recommendation, let alone demand.
In v5 I've gone with neither approach, and instead spun off a helper
method qemu_open_cloexec which I think is clearer than both.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
