Skip to content

cpu/native: do not exit when real_read returns 0#2686

Merged
cgundogan merged 1 commit intoRIOT-OS:masterfrom
cgundogan:native_tap_read_zero
Apr 7, 2015
Merged

cpu/native: do not exit when real_read returns 0#2686
cgundogan merged 1 commit intoRIOT-OS:masterfrom
cgundogan:native_tap_read_zero

Conversation

@cgundogan
Copy link
Copy Markdown
Member

When real_read() returns 0 bytes, the application exits with an error. Posix states, that 0 indicates EOF and is not really an error case. So I propose to remove this else-check to not end the execution of native in such a case. See #2664

@cgundogan cgundogan added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Mar 22, 2015
@LudwigKnuepfer
Copy link
Copy Markdown
Member

EOF on any of natives file handles is an error.

@LudwigKnuepfer
Copy link
Copy Markdown
Member

Unless you can explain what EOF means on a tap device and why it isn't an error, I'd reject this PR.

@OlegHahm OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@cgundogan
Copy link
Copy Markdown
Member Author

Well the underlying (native) read operation does not return an error, since 0 is not listed as an error value in the posix documentation.. My vote would be to also continue with the read loop upon receiving a return value of 0. I have no idea why I am the only person, who has this behavior, but it is really annoying to use native when it crashes ever so often (;

The read operation is blocking, isn't it? Is there a timeout for this read operation to return with 0, or will this block infinitely?

@LudwigKnuepfer
Copy link
Copy Markdown
Member

read should only be called if data is available in the first place due to the interrupt based implementation around it. It should be non-blocking due to this as well. Anyway - this interrupt/poll based program flow is why I think zero is an error to begin with. It might be some strange and harmless race condition though.

I would suggest you check the tuntap implementations of Linux and BSD and find out why zero is returned.

Finally, I searched other tap users quickly and I think it was virtualbox who also ignored EOF on taps, so I expect it is in fact harmless.

@LudwigKnuepfer
Copy link
Copy Markdown
Member

For the record: with #2558 I do get this error as well if I run ifconfig in the test on an interface which has been freshly created on the host.

@LudwigKnuepfer
Copy link
Copy Markdown
Member

After reading the Linux implementation, I ACK ignoring the event, but not this implementation.
Please make it:

+    else if (nread == 0) {
+        DEBUG("_native_handle_tap_input: ignoring null-event");
+    }

@cgundogan cgundogan force-pushed the native_tap_read_zero branch from aa68140 to 4fae22b Compare April 6, 2015 08:19
@cgundogan
Copy link
Copy Markdown
Member Author

addressed @LudwigOrtmann's comment

@LudwigKnuepfer
Copy link
Copy Markdown
Member

ACK

@cgundogan
Copy link
Copy Markdown
Member Author

@LudwigOrtmann and Travis are happy => GO

cgundogan added a commit that referenced this pull request Apr 7, 2015
cpu/native: do not exit when real_read returns 0
@cgundogan cgundogan merged commit 095bd32 into RIOT-OS:master Apr 7, 2015
@cgundogan cgundogan deleted the native_tap_read_zero branch April 8, 2015 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants