Skip to content

Conversation

@luca3m
Copy link
Contributor

@luca3m luca3m commented May 24, 2017

This is done to reduce memory usage

@mstemm
Copy link
Contributor

mstemm commented Jun 5, 2017

While you're adding checks for evt->m_tinfo being NULL, I think there are a few more parsing functions that need the check:

  • sinsp_parser::parse_openat_dir
  • sinsp_parser::parse_open_openat_creat_exit (there is an ASSERT, but an explicit exit in non debug builds would be useful)
  • sinsp_parser::add_socket
  • sinsp_parser::parse_close_exit
  • sinsp_parser::parse_tracer (again, just an ASSERT)
  • sinsp_parser::parse_rw_exit
  • sinsp_parser::parse_fchdir_exit
  • sinsp_parser::parse_getrlimit_setrlimit_exit

None of these are regressions, so I'm ok with filing an issue to track it and doing it later.

Otherwise, lgtm assuming tests pass!

@luca3m luca3m merged commit 41bb370 into dev Jun 5, 2017
@luca3m luca3m deleted the mem-fix branch June 5, 2017 22:45
mstemm pushed a commit that referenced this pull request Jun 7, 2017
* Set environment variables only on main threads

* Set cwd only on main thread, it causes extra thread lookups during startup

* Add few sanity checks for the presence of tinfo

* more of the previous

* more of the previous

* Replace all access to m_env with a call to the getter get_env()

* Other nullptr tinfo checks

* Add other nullptr checks
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.

3 participants