Skip to content

shell: fix netif command for return values#2736

Merged
Lotterleben merged 1 commit intoRIOT-OS:masterfrom
miri64:shell/fix/netif-return
Apr 23, 2015
Merged

shell: fix netif command for return values#2736
Lotterleben merged 1 commit intoRIOT-OS:masterfrom
miri64:shell/fix/netif-return

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Mar 29, 2015

If we have return values, we should utilize them :-)

@miri64 miri64 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 NSTF labels Mar 29, 2015
@miri64 miri64 added this to the Network Stack Task Force milestone Mar 29, 2015
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but this bugged me :D

@miri64 miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 29, 2015
@OlegHahm OlegHahm force-pushed the master branch 3 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@miri64 miri64 force-pushed the shell/fix/netif-return branch from 195e08d to 418a75a Compare April 6, 2015 15:36
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Apr 6, 2015

Rebased to current master

@miri64 miri64 assigned OlegHahm and unassigned haukepetersen Apr 9, 2015
@miri64 miri64 force-pushed the shell/fix/netif-return branch from 9358bf4 to 96a6855 Compare April 10, 2015 06:16
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Apr 10, 2015

Rebased tu current master

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure you want to remove the help text for set state?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

@OlegHahm OlegHahm assigned LudwigKnuepfer and unassigned OlegHahm Apr 15, 2015
@miri64 miri64 assigned Lotterleben and unassigned LudwigKnuepfer Apr 21, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Apr 21, 2015

I'll pass the hot potato along to get this finally merged.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just wondering, is there a reason you're not returning EINVAL here (an other errnos in the rest of the document)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we never established to (nor did I ever heard of) return of errnos in exit status of shell commands.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the other shell commands don't. Also, since the value of the errnos is highly dependent on the platform and the user will only see the value and not the macro (when calling echo "$?" in later shell implementations e.g.) I don't really see the point.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought at least some values are specified by POSIX (but maybe it's just the name). Also, the shell doesn't necessarily use a plaintext protocol for I/O. However, I don't want to advocate for the use of errno values. Just wanted to point out that it has been used in RIOT context - though not in RIOT itself - for this purpose.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification :)

@miri64 miri64 force-pushed the shell/fix/netif-return branch from b216b0f to 713c211 Compare April 21, 2015 15:48
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Apr 21, 2015

Rebased to current master.

@miri64 miri64 force-pushed the shell/fix/netif-return branch from 713c211 to a099095 Compare April 22, 2015 21:34
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Apr 22, 2015

Rebased to current master and squashed

@miri64 miri64 removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Apr 22, 2015
@Lotterleben
Copy link
Copy Markdown
Member

kicked travis

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Apr 23, 2015

Travis likes it.

@Lotterleben
Copy link
Copy Markdown
Member

Wow, that was fast. Then let's ACK and Go :)

Lotterleben added a commit that referenced this pull request Apr 23, 2015
shell: fix netif command for return values
@Lotterleben Lotterleben merged commit e6e45ad into RIOT-OS:master Apr 23, 2015
@miri64 miri64 deleted the shell/fix/netif-return branch April 23, 2015 08:21
@miri64 miri64 added the Area: network Area: Networking label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking 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.

5 participants