On Wed, Oct 18, 2017 at 1:42 AM, <b...@elotl.co> wrote:
> As a way of getting up to speed on the codebase, finish up the remaining
> work on issue #904. Add new exception type and catch that where necessary.
>
If you put the text "Fixes #904" somewhere inside the commit message,
github will automatically close this issue when your patch is committed.
Very useful, so please do this.
> Let me know if you think this warrants a new test or if you have questions.
>
Thanks! While tests are always welcome, I don't think in this case it's
critical, and I'll commit a patch without one.
I have one more trivial request below, if you can fix it and send a new
patch, I'll commit it. Thanks!
> Signed-off-by: Brendan Cox <b...@elotl.com>
>
> ---
> core/app.cc | 5 ++++-
> core/elf.cc | 20 ++++++++++----------
> core/osv_execve.cc | 7 +++++--
> include/osv/app.hh | 6 ++++++
> 4 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/core/app.cc b/core/app.cc
> index 527a552..4c8612a 100644
> --- a/core/app.cc
> +++ b/core/app.cc
> @@ -170,7 +170,10 @@ application::application(const std::string& command,
>
> merge_in_environ(new_program, env);
> _lib = current_program->get_library(_command);
> - } catch(const std::exception &e) {
> + }
>
Please keep the "catch" on the same line (as before this patch), and please
make sure you use 4 spaces, no tab characters, for indentation.
> + catch(const launch_error &e) {
> + throw;
> + } catch(const std::exception &e) {
> throw launch_error(e.what());
> }
>
> diff --git a/core/elf.cc b/core/elf.cc
> index c53cbbb..2a46633 100644
> --- a/core/elf.cc
> +++ b/core/elf.cc
> @@ -232,27 +232,27 @@ void file::load_elf_header()
> try {
> read(0, &_ehdr, sizeof(_ehdr));
> } catch(error &e) {
> - throw std::runtime_error(
> + throw osv::invalid_elf_error(
> std::string("can't read elf header: ") +
> strerror(e.get()));
> }
> if (!(_ehdr.e_ident[EI_MAG0] == '\x7f'
> && _ehdr.e_ident[EI_MAG1] == 'E'
> && _ehdr.e_ident[EI_MAG2] == 'L'
> && _ehdr.e_ident[EI_MAG3] == 'F')) {
> - throw std::runtime_error("bad elf header");
> + throw osv::invalid_elf_error("bad elf header");
> }
> if (!(_ehdr.e_ident[EI_CLASS] == ELFCLASS64)) {
> - throw std::runtime_error("bad elf class");
> + throw osv::invalid_elf_error("bad elf class");
> }
> if (!(_ehdr.e_ident[EI_DATA] == ELFDATA2LSB)) {
> - throw std::runtime_error("bad elf endianness");
> + throw osv::invalid_elf_error("bad elf endianness");
> }
> if (!(_ehdr.e_ident[EI_VERSION] == EV_CURRENT)) {
> - throw std::runtime_error("bad elf version");
> + throw osv::invalid_elf_error("bad elf version");
> }
> if (!(_ehdr.e_ident[EI_OSABI] == ELFOSABI_LINUX
> || _ehdr.e_ident[EI_OSABI] == 0)) {
> - throw std::runtime_error("bad os abi");
> + throw osv::invalid_elf_error("bad os abi");
> }
> // We currently only support running ET_DYN objects (shared library or
> // position-independent executable). In the future we can add support
> for
> @@ -260,7 +260,7 @@ void file::load_elf_header()
> // loading them at their specified address and moving the kernel out
> of
> // their way.
> if (_ehdr.e_type != ET_DYN) {
> - throw std::runtime_error(
> + throw osv::invalid_elf_error(
> "bad executable type (only shared-object or PIE
> supported)");
> }
> }
> @@ -270,7 +270,7 @@ void file::read(Elf64_Off offset, void* data, size_t
> size)
> // read(fileref, ...) is void, and crashes with assertion failure if
> the
> // file is not long enough. So we need to check first.
> if (::size(_f) < offset + size) {
> - throw std::runtime_error("executable too short");
> + throw osv::invalid_elf_error("executable too short");
> }
> ::read(_f, data, offset, size);
> }
> @@ -432,7 +432,7 @@ void object::load_segments()
> break;
> default:
> abort();
> - throw std::runtime_error("bad p_type");
> + throw osv::invalid_elf_error("bad p_type");
> }
> }
> // As explained in issue #352, we currently don't correctly support
> TLS
> @@ -520,7 +520,7 @@ Elf64_Dyn& object::dynamic_tag(unsigned tag)
> {
> auto r = _dynamic_tag(tag);
> if (!r) {
> - throw std::runtime_error("missing tag");
> + throw osv::invalid_elf_error("missing tag");
> }
> return *r;
> }
> diff --git a/core/osv_execve.cc b/core/osv_execve.cc
> index f4505cf..8a8ae7a 100644
> --- a/core/osv_execve.cc
> +++ b/core/osv_execve.cc
> @@ -46,8 +46,11 @@ static int thread_run_app_in_namespace(std::string
> filename,
> app_status.exit_code = app->get_return_code();
> debugf_execve("thread_run_app_in_namespace ret = %d tid=%ld\n",
> app_status.exit_code, tid);
> }
> - catch (osv::launch_error &ex) {
> - app_status.errno_code = ENOENT; // Assume the only possible
> problem is "no such file"
> + catch (osv::invalid_elf_error &ex) {
> + app_status.errno_code = ENOEXEC;
> + app_status.exit_code = -1;
> + } catch (osv::launch_error &ex) {
> + app_status.errno_code = ENOENT; // Assume all the other errors
> are "no such file"
> app_status.exit_code = -1;
> }
>
> diff --git a/include/osv/app.hh b/include/osv/app.hh
> index 83e017e..1a39e32 100644
> --- a/include/osv/app.hh
> +++ b/include/osv/app.hh
> @@ -38,6 +38,12 @@ public:
> launch_error(std::string msg) : std::runtime_error(msg) {}
> };
>
> +class invalid_elf_error : public launch_error
> +{
> +public:
> + invalid_elf_error(std::string msg) : launch_error(msg) {}
> +};
> +
> class multiple_join_error : public std::runtime_error
> {
> public:
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
