Clean pr26 utility updates

[JensLorenz]

This is a rework of the PR26. It separates the different modifications provided by @GeniviAMmaintainer in single commits. Moreover, it is an entire rework of different aspects considered in the PR.

In addition there are new checks in the gtests introduced to ensure in future that PRs done by others will not break the current implementation. One example to mention here is the revents problem.

The only modification missing is the thread_id check. First we should clearly understand the background of the modification therefore further clarification with the developer(s) is needed.

[gunnarx]

Ack. I will try to find time the next few days to review.

[GeniviAMmaintainer]

Looks good, definetly an performance boost. We had some troube with disabled timerfd flag, we are investigating further.

[JensLorenz]

@GeniviAMmaintainer: I investigated the issue WITH_TIMERFD=OFF. Actually figured out some troubles. There will be an update of the PR. On our side issues are solved. Need some additional tests and contribute hopefully today.

[gunnarx]

Can we get a bit shorter PRs? :-) Seriously, there's too much going on in one PR here. OK, there were many comments, here is kind of a quick summary:

ac61215 Dlt init function returns now correctly

Nothing major. LGTM.

5d05984 Common API Wrapper

It's a little hard to understand this PR. PR26 seems to be merged, but this is a "rework" of PR26. It seems to be additional commits on top of PR26 (and a few more commits that have also been merged since then).

So am I looking at a rework of the same changes, or brand new changes? This commit confuses me because it has the same flaw as some in PR26, namely that it mixes whitespace changes with actual code changes. So that made me think I was looking at some kind of resend of PR26, but looking at the commits I suppose that is not the case.

The commit comment does not mention that whitespace was intended to be fixed up.

Anyway, I ignored what seemed to be whitespace changes (can't guarantee that the code was not changed because I didn't look that hard), and looked at what seemed to be code changes. Some of the timer handling I admit I don't know well enough to make judgement on.

--> Overall, I'd say OK.

1a6dfa0 Test: Add macro to increase the loop count of serializer tests

Test code. Loop count increased. OK - LGTM

97737f9 Update option message

Nothing special. LGTM

aa91e78 Test: Value range of mExpected not sufficient on 32bit architecture

Nitpick: Commit comment summary line is too long. Could be: AMUtil: Test: Fix mExpected value range for 32-bit Otherwise it's all good.

--> OK (ish)

186b11b Test: Remove off-by-one problem for timer meas. and socket tests

I first reacted to putting active code (function call) inside of assert macro, which is of course a no-no, but here in test code it makes sense. Because of the repetition, maybe worth to refactor into a wrapper ASSERT_E_OK() or ASSERT_SUCCESS(), but that's a matter of opinion.

--> OK

02c21c4 Test: Lets check the return value of addTimer

Similar test code.
LGTM.

5978939 Test: Ensure that revents are tested in fire callback

LGTM.

0aa2b1d Improve actionPoll lambda implementation for signal and timer

Is read() really handled correctly?

--> Requested changes and/or explanation. (Or you can put this on a later TODO of course)

87c0b9a Rework of listenToSignal and minor improvements

A few comments, but that's more to consider for future. Concerns about erase() pattern

--> Requested change

02fe4f2 Improve timer implementation and ensure that no fd leak happens

Some concerns about #ifdef -should they remain? Also more comments on the right pattern for erase() in a loop I don't think it MUST be changed now, but the whole code base should have a consistently followed pattern, IMHO

--> OK (for now)

ab3e7e9 Change from mPipe to eventfd

Same comments as in 0aa2b1d regarding read() and also write()

--> as before

2ac03e8 Rework of socketHandler to avoid calls of invalidated objects

This is one of the things that seem to be a major change compared to PR26. So... it would basically stand on its own, right? I guess I just feel these "PRs" include so much and seem to not be isolated to one thing...

I'm uneasy about the assumptions made about one container being at least as large as the other. (There is no boundary check for the iteration over the polling Array)

Seems like some asserts should help clarify the intention & assumptions, and also check those assumptions during runtime, but I don't know if you use asserts (outside of tests).

--> Change (or at least comment) requested

bb06746 ONLY timers will be closed in worker thread

Some API style comments. --> Change requested (but it's minor really. it's just style)

b8d79d2 Rework of exception debug messages

The commit comment could be a bit better than "Rework" messages At least try to show to a reader that this is a harmless change (because other "rework" has been MAJOR rewrites!)

Why was one log message removed? The rest looks fine.

--> ?

355bb7c Cleanup indents and whitespaces

Yes it looks like only whitespace was changed :- --> OK

30c699f Fix startup sequence addFdPoll and removeFdPoll

Polling code that I can't confidently comment on. (I have not yet fully understood the strategy around the polling) Sorry, no time to dig deeper.

--> INCOMPLETE REVIEW!

a2af7c9 Wakeup of ppoll is now also triggered on addFdPoll

Polling code that I can't confidently comment on, but looks OK as far as I can see. --> OK / unsure

[JensLorenz]

@gunnarx: Sorry, wasn't aware that an update breaks all the comments here. However, most of the findings are addressed now - even if there were more likely minor issues. Others are still readable.

BTW: Following patches are newly added:

ab3eea9: AMUtil: Store signalfd as file descriptor instead of pollfd handle 95dc06f: AMUtil: Change log level of removeFDPoll from error to warning bfeec4b: AMUtil: No check callback needed for eventfd and signalfd

[gunnarx]

Unfortunately I don't have time to re-review the changes to check for any bugs, but I'm happy that it seemed to be useful and some changes were made to improve the design. I am sure we can address anything that is remaing in future rework.

[JensLorenz]

@GeniviAMmaintainer: I have pushed two new patch files. We have identified that the unit test for the routing and command interface fails. 5c98a24 solves the problem. Hope we can merge tomorrow.

[GeniviAMmaintainer]

I still don't have the final results for our tests. Let's wait for the next meeting for the merge.

[JensLorenz]

@GeniviAMmaintainer : Please merge as discussed.