Wireshark-dev: Re: [Wireshark-dev] [Wireshark-commits] master 43a81b6: Add some information on
On Thursday 31 July 2014 16:40:53 Guy Harris wrote:
> On Jul 31, 2014, at 3:11 PM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:
[..]
> > Oh my, that filesystem.c code is really ugly and relying on a lot of
> > assumptions. Why does it need to distinguish build dirs from other dirs in
> > the first place?
>
> So that you can just type "./wireshark" or "./tshark" after you've done a
> build, and have it Just Work, rather than having to install Wireshark or
> TShark before you can run it. Note that we run TShark to generate some man
> pages.
The binaries themselves already Just Work(tm) without libtool because CMake
sets RPATH. (If it still tries to search for globally installed WS, note that
there existed a bug in CMake 2.8[1] that set the wrong RPATH when linked
against libraries in `/lib64`. That got fixed in CMake 3.0)
[1]: http://www.cmake.org/Bug/view.php?id=14875
> > From the comments, it seems to that for security/stability
> > reasons,
>
> From one of the comments:
>
> /*
> * Check whether WIRESHARK_RUN_FROM_BUILD_DIRECTORY is set in the
> * environment; if so, set running_in_build_directory_flag if we
> * weren't started with special privileges. (If we were started
> * with special privileges, it's not safe to allow the user to point
> * us to some other directory; running_in_build_directory_flag, when
> * set, causes us to look for plugins and the like in the build
> * directory.)
> */
>
> So the code that sets the "running from the build directory flag" takes into
> account security issues, but it doesn't *exist* for security reasons.
>
> On the other hand, from doc/README.packaging:
>
> WIRESHARK CONTAINS OVER TWO MILLION LINES OF SOURCE CODE. DO NOT RUN THEM
> AS ROOT.
That was indeed the reason for comments on security reasons, together with
ignoring plugins from the env vars. Fun fact: currently there are over 3.5M
lines of code (only counting *.h and *.c, surely I missed some cpp and py
files). I would like to get rid of the WIRESHARK_RUN_FROM_BUILD_DIRECTORY if
possible, not treating it specially and instead rely on environment variables
to configure stuff.
> > and another reason is to make plugins get loaded from the build dir.
>
> Not just plugins, but also global configuration files, etc..
Currently the configuration cannot be overridden when ran from the confdir,
but that seems to be a useless restriction. Setting WIRESHARK_DATA_DIR should
work without surprises (this behavior is not even documented, only root/setuid
should have this special behavior).
> > What about solely relying on envvars?
>
> Sure, as long as the user doesn't have to do anything to set the environment
> variable. Otherwise, it doesn't Just Work.
> > Then there can be a shell-script if you
> > like the wrapper provided by libtool:
> >
> > #!/bin/bash
> > # tools/run.sh - Wrapper for binaries
>
> > # since Mac does not have `readlink -f`, this is an alternative:
> Neither do older versions of other BSD-flavored OSes. I guess Apple just
> haven't updated readlink in a while.
Apple has a greadlink ("GNU readlink"), but I don't think that is installed by
default either. Actually, since `$0` is expected to be a symlink, it should be
fine to use just `readlink "$0"`, and exit in other cases (printing a help
message instead?).
> > rundir=$(cd "$(dirname "$0")" && pwd)
> > #rundir=$(dirname "$(readlink -f "$0")")
> > export WIRESHARK_DATA_DIR=$rundir
> > export WIRESHARK_PLUGIN_DIR=$rundir
> > #etc.
> > exec "$rundir/${0##*/}" "$@"
> >
> > With links:
> > tmpbin/tshark -> ../tools/run.sh
> > tmpbin/wireshark -> ../tools/run.sh
> > etc.
>
> So tmpbin is the top-level source directory, so running "./tshark",
> "./wireshark", etc. from the top-level CMake build directory would be
> sufficient?
The object (output) directory. For top-level directories, the symlink would
appear as `./tshark -> tools/run.sh` (assuming that run.sh is copied over). If
this approach is taken, an option must be added to support the use of GDB /
valgrind.
Kind regards,
Peter
https://lekensteyn.nl
