Implementation of HANDLE_EINTR macro using standard C++ (without gcc extensions)

432 views
Skip to first unread message

Alex

unread,
Sep 13, 2016, 5:11:06 PM9/13/16
to Chromium-dev
Currently HANDLE_EINTR is implemented as follows:

#define HANDLE_EINTR(x) ({ \
  decltype(x) eintr_wrapper_result; \
  do { \
    eintr_wrapper_result = (x); \
  } while (eintr_wrapper_result == -1 && errno == EINTR); \
  eintr_wrapper_result; \
})

which uses gcc extension ({..}) to evaluate multiple statements and return the value of the last one. This makes Visual Studio's IntelliSense very unhappy. How about implementing it using a lambda instead? This becomes standard C++ and can be parsed by a compliant compiler just fine. Here is an example implementation of the above:

#define HANDLE_EINTR(x) ([&]() -> decltype(x) { \
  decltype(x) eintr_wrapper_result; \
  do { \
    eintr_wrapper_result = (x); \
  } while (eintr_wrapper_result == -1 && errno == EINTR); \
  return eintr_wrapper_result; \
})()

Any thoughts?

Alex

Nico Weber

unread,
Sep 13, 2016, 5:21:58 PM9/13/16
to Alex Vakulenko, Chromium-dev
That's in a #if defined(OS_POSIX) block, how does intellisense get there?

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Markus Gutschke (顧孟勤)

unread,
Sep 13, 2016, 5:52:03 PM9/13/16
to avaku...@chromium.org, Chromium-dev
Historically, this code needed to work in both C and C++ mode. So, using GCC extensions made a lot of sense. After all, it only ever is needed on platforms that are built with GCC or GCC-compatible compilers (EINTR is only meaningful on POSIX, as far as I can tell).

It looks as if this requirement is no longer present, as the code now uses "decltype" instead of GCC's "typeof". So, that's a bit of good news. And maybe your change wouldn't break anything that isn't already broken.

Originally, and for maximum compatibility though, the macro was modeled to look very similar to TEMP_FAILURE_RETRY() in <unistd.h>. In fact, we probably should have just used that standard macro instead. But that ship has long since sailed.


Markus


--

Alex

unread,
Sep 13, 2016, 5:55:14 PM9/13/16
to Chromium-dev, avaku...@chromium.org
That's in a #if defined(OS_POSIX) block, how does intellisense get there?

Well, that's actually from libchrome library (port of Chromium's base lib to Android). I just use Visual Studio to edit the code, POSIX or not :)

Adam Rice

unread,
Sep 14, 2016, 1:05:32 AM9/14/16
to avaku...@chromium.org, Chromium-dev
That macro is fairly performance-critical. Checking that GCC on ARM produces comparable quality machine code output would be a good idea before committing to a change.

Random bikeshedding: if you use a lambda to wrap the expression "x", then the rest can be implemented as a template function.

On 14 September 2016 at 06:55, Alex <avaku...@chromium.org> wrote:
That's in a #if defined(OS_POSIX) block, how does intellisense get there?

Well, that's actually from libchrome library (port of Chromium's base lib to Android). I just use Visual Studio to edit the code, POSIX or not :)

--

Yuta Kitamura

unread,
Sep 14, 2016, 1:33:37 AM9/14/16
to avaku...@chromium.org, Chromium-dev
This doesn't have to be a macro (well, I didn't test this at all, so it may not compile):

template <typename SysCall, typename... Args>
auto HandleEintr(SysCall sys_call, Args... args)
    -> decltype(sys_call(args...)) {
  decltype(sys_call(args...)) result;
  do {
    result = sys_call(args...);
  } while (result == -1 && errno == EINTR);
  return result;
}

Usage:

void* buf = ...;
size_t count = ...;
ssize_t result = HandleEintr(read, buf, count);

--

Alex Vakulenko

unread,
Sep 14, 2016, 10:39:19 AM9/14/16
to Yuta Kitamura, Chromium-dev
Yes, I considered that, if for a fact that stuff like HANDLE_EINTR(foo(bar++)) has really undesired side-effect of incrementing bar on each retry and template function is safe for this, but since the call semantic of the template function is very different from the old macro, I didn't originally propose it.
Reply all
Reply to author
Forward
0 new messages