Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builtin exit #125

Merged
merged 7 commits into from
Apr 11, 2017
Merged

Builtin exit #125

merged 7 commits into from
Apr 11, 2017

Conversation

n0izn0iz
Copy link
Contributor

 ✘ n0iz@nomad  ~/Projects/42sh   builtin_exit  ./42sh
[n0iz@nomad ~/Projects/42sh]$ ./42sh
[n0iz@nomad ~/Projects/42sh]$ exit 42
  executing builtin exit
[n0iz@nomad ~/Projects/42sh]$ exit
  executing builtin exit
 ✘ n0iz@nomad  ~/Projects/42sh   builtin_exit  echo $?
42

@n0izn0iz
Copy link
Contributor Author

n0izn0iz commented Apr 10, 2017

c'est le retour du random travis fail :/ https://travis-ci.org/42shpimanmls/42sh/builds/220715109
a voir après le merge de #118

@n0izn0iz n0izn0iz mentioned this pull request Apr 10, 2017
@M5oul
Copy link
Contributor

M5oul commented Apr 11, 2017

Doesn't works on my side.

return (*get_ptr());
}

void set_last_exit_status(t_uchar status)
Copy link
Contributor

Choose a reason for hiding this comment

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

il faut update la variable status dans les variable locale non pour l'export?

Copy link
Contributor Author

@n0izn0iz n0izn0iz Apr 11, 2017

Choose a reason for hiding this comment

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

oui pardon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

enfin, c'est une variable locale mais elle est pas exportée automatiquement

Copy link
Contributor

Choose a reason for hiding this comment

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

c'est un variable locale non exporte

Copy link
Contributor

Choose a reason for hiding this comment

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

c'est une variable du shell quoi.

@n0izn0iz
Copy link
Contributor Author

What doesn't work @M5oul ??

@M5oul
Copy link
Contributor

M5oul commented Apr 11, 2017

Following the example on first post.
I don't get 42 at output status.

@n0izn0iz
Copy link
Contributor Author

Make re? can you post log?

@lsimonne
Copy link
Contributor

➜  42sh git:(builtin_exit) ./42sh
[ChezLouise@ ~/Documents/42/42sh]$ exit foo
  executing builtin exit
➜  42sh git:(builtin_exit) echo $?
0
bash-3.2$ exit foo
exit
bash: exit: kj: numeric argument required
➜  42sh git:(builtin_exit) echo $?
255

sinon chez moi le reste fonctionne

@n0izn0iz
Copy link
Contributor Author

n0izn0iz commented Apr 11, 2017

If n is specified, but its value is not between 0 and 255 inclusively, the exit status is undefined.

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#exit

@lsimonne
Copy link
Contributor

ok!
par contre faudra faire les valeurs de retour des commandes en cas d'erreur -> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02

@n0izn0iz
Copy link
Contributor Author

yes!

@M5oul
Copy link
Contributor

M5oul commented Apr 11, 2017

Ok, it works. I don't know why it didn't works.

@n0izn0iz
Copy link
Contributor Author

 ✘ n0iz@nomad  ~/Projects/42sh   builtin_exit ●  ./42sh
[n0iz@nomad ~/Projects/42sh]$ ./42sh
[n0iz@nomad ~/Projects/42sh]$ exit 42
  executing builtin exit
[n0iz@nomad ~/Projects/42sh]$ env | grep ?
?=42
[n0iz@nomad ~/Projects/42sh]$ 

@n0izn0iz
Copy link
Contributor Author

 ✘ n0iz@nomad  ~/Projects/42sh   builtin_exit ●  ./42sh
[n0iz@nomad ~/Projects/42sh]$ ./42sh
[n0iz@nomad ~/Projects/42sh]$ exit 42
  executing builtin exit
[n0iz@nomad ~/Projects/42sh]$ set | grep ?
    executing builtin set
    done executing builtin set, ok
?=42
[n0iz@nomad ~/Projects/42sh]$ 

@pilespin
Copy link
Contributor

command not found must be 127

@n0izn0iz
Copy link
Contributor Author

ça c'est pas a exit de s'en occuper

@lsimonne
Copy link
Contributor

techniquement, c'est pas un problème d'exit, mais d'exécution (cf mon commentaire au-dessus)

@n0izn0iz
Copy link
Contributor Author

Yay!

@n0izn0iz n0izn0iz merged commit 8d65938 into master Apr 11, 2017
@n0izn0iz n0izn0iz deleted the builtin_exit branch April 11, 2017 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants