Skip to content

dist/tools/*/start_network.sh: ensure TUN/TAP interface#13801

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
miri64:tools/fix/start_network-interface-init
Apr 8, 2020
Merged

dist/tools/*/start_network.sh: ensure TUN/TAP interface#13801
benpicco merged 1 commit intoRIOT-OS:masterfrom
miri64:tools/fix/start_network-interface-init

Conversation

@miri64
Copy link
Member

@miri64 miri64 commented Apr 3, 2020

Contribution description

Previously, when the creation of the TUN/TAP interface failed in one of the start_network.sh scripts, the script will fail with a cryptic error like

dist/tools/ethos/start_network.sh: 68: [: -eq: unexpected operator

This fix ensures, that the value of this variable checked is always set such that in the error case, ethos/sliptty won't start.

Testing procedure

Ask @benpicco. I was not able to reproduce it.

Issues/PRs references

See #13734 (comment)

Previously, when the creation of the TUN/TAP interface failed in one of
the `start_network.sh` scripts, the script will fail with a cryptic
error like

> dist/tools/ethos/start_network.sh: 68: [: -eq: unexpected operator

This fix ensures, that the value of this variable checked is always set
such that in the error case, `ethos`/`sliptty` won't start.
@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tools Area: Supplementary tools CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Apr 3, 2020
@miri64 miri64 added this to the Release 2020.04 milestone Apr 3, 2020
@miri64 miri64 requested a review from benpicco April 3, 2020 11:48
@benpicco
Copy link
Contributor

benpicco commented Apr 3, 2020

I don't see how that would help - START_ETHOS will be overwritten in any case

@leandrolanzieri
Copy link
Contributor

I don't see how that would help - START_ETHOS will be overwritten in any case

Then, this does not fix your problem? Shall we close?

@miri64
Copy link
Member Author

miri64 commented Apr 8, 2020

I don't see how that would help - START_ETHOS will be overwritten in any case

Then, this does not fix your problem? Shall we close?

I don't understand @benpicco's argument to be honest. The error comes because if create_tap fails, START_ETHOS/START_SLIP won't be initialized, having an empty string for numerical comparison. This fix initializes both with a default value, that says "don't start ethos/slip".

@benpicco
Copy link
Contributor

benpicco commented Apr 8, 2020

The error comes because if create_tap fails

Ah! I overlooked the && after create_tap, so the if only gets evaluated if that succeeds.

In that case, the patch is the proper thing to do.

@benpicco
Copy link
Contributor

benpicco commented Apr 8, 2020

But shouldn't it be START_SLIP=0 so slip does not get started when the interface can not be created?

@miri64
Copy link
Member Author

miri64 commented Apr 8, 2020

But shouldn't it be START_SLIP=0 so slip does not get started when the interface can not be created?

START_SLIP=0 would mean it gets started... the value check is for a return code.

@benpicco benpicco merged commit ecd266f into RIOT-OS:master Apr 8, 2020
@miri64 miri64 deleted the tools/fix/start_network-interface-init branch April 8, 2020 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants