| Message ID | 20200427031713.14013-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Mon, Apr 27, 2020 at 06:17:04AM +0300, Laurent Pinchart wrote: > Similarly to libcameraBuildPath(), there's a need to locate resources > within the source tree when running libcamera without installing it. > Support this use case with a new utils::libcameraSourcePath() function. > > The implementation currently leaks the path to the source tree in the > installed binary. The data is put in a separate section named .zero > which will need to be zeroed at installation time to fix this. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > meson.build | 2 ++ > src/libcamera/include/utils.h | 1 + > src/libcamera/utils.cpp | 37 ++++++++++++++++++++++++++++++++--- > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/meson.build b/meson.build > index c6e6a934e54e..2058a6a77ceb 100644 > --- a/meson.build > +++ b/meson.build > @@ -26,6 +26,8 @@ libcamera_version = libcamera_git_version.split('+')[0] > cc = meson.get_compiler('c') > config_h = configuration_data() > > +config_h.set('LIBCAMERA_SOURCE_DIR', '"' + meson.current_source_dir() + '"') > + > if cc.has_header_symbol('execinfo.h', 'backtrace') > config_h.set('HAVE_BACKTRACE', 1) > endif > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h > index 242eeded9d50..3334ff16613d 100644 > --- a/src/libcamera/include/utils.h > +++ b/src/libcamera/include/utils.h > @@ -188,6 +188,7 @@ private: > details::StringSplitter split(const std::string &str, const std::string &delim); > > std::string libcameraBuildPath(); > +std::string libcameraSourcePath(); > > } /* namespace utils */ > > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > index 97f9b632e45b..056a67e93e0f 100644 > --- a/src/libcamera/utils.cpp > +++ b/src/libcamera/utils.cpp > @@ -360,9 +360,10 @@ bool isLibcameraInstalled() > * > * During development, it is useful to run libcamera binaries directly from the > * build directory without installing them. This function helps components that > - * need to locate resources, such as IPA modules or IPA proxy workers, by > - * providing them with the path to the root of the build directory. Callers can > - * then use it to complement or override searches in system-wide directories. > + * need to locate resources in the build tree, such as IPA modules or IPA proxy > + * workers, by providing them with the path to the root of the build directory. > + * Callers can then use it to complement or override searches in system-wide > + * directories. > * > * If libcamera has been installed, the build directory path is not available > * and this function returns an empty string. > @@ -385,6 +386,36 @@ std::string libcameraBuildPath() > return dirname(info.dli_fname) + "/../../"; > } > > +namespace { > +/* \todo Figure out a way to zero this section at install time */ > +__attribute__((section(".zero"))) > +const char sourceDirectory[] = LIBCAMERA_SOURCE_DIR; > +} /* namespace */ > + > +/** > + * \brief Retrieve the path to the source directory > + * > + * During development, it is useful to run libcamera binaries directly from the > + * build directory without installing them. This function helps components that > + * need to locate resources in the source tree, such as IPA configuration > + * files, by providing them with the path to the root of the source directory. > + * Callers can then use it to complement or override searches in system-wide > + * directories. > + * > + * If libcamera has been installed, the source directory path is not available > + * and this function returns an empty string. > + * > + * \return The path to the source directory if running from a build, or an empty "from a build" or "from the build directory" ? Nit apart: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > + * string otherwise > + */ > +std::string libcameraSourcePath() > +{ > + if (isLibcameraInstalled()) > + return std::string(); > + > + return std::string(sourceDirectory) + "/"; > +} > + > } /* namespace utils */ > > } /* namespace libcamera */ > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On 27/04/2020 04:17, Laurent Pinchart wrote: > Similarly to libcameraBuildPath(), there's a need to locate resources > within the source tree when running libcamera without installing it. > Support this use case with a new utils::libcameraSourcePath() function. Eeeep. > The implementation currently leaks the path to the source tree in the > installed binary. The data is put in a separate section named .zero > which will need to be zeroed at installation time to fix this. Indeed. I guess we can't get any of this from the debug symbols can we? That won't be reliable. > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This is always going to be painful... but I don't think we should block this branch just because of leaking the source path - but we certainly want to fix it soon after. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > meson.build | 2 ++ > src/libcamera/include/utils.h | 1 + > src/libcamera/utils.cpp | 37 ++++++++++++++++++++++++++++++++--- > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/meson.build b/meson.build > index c6e6a934e54e..2058a6a77ceb 100644 > --- a/meson.build > +++ b/meson.build > @@ -26,6 +26,8 @@ libcamera_version = libcamera_git_version.split('+')[0] > cc = meson.get_compiler('c') > config_h = configuration_data() > > +config_h.set('LIBCAMERA_SOURCE_DIR', '"' + meson.current_source_dir() + '"') > + > if cc.has_header_symbol('execinfo.h', 'backtrace') > config_h.set('HAVE_BACKTRACE', 1) > endif > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h > index 242eeded9d50..3334ff16613d 100644 > --- a/src/libcamera/include/utils.h > +++ b/src/libcamera/include/utils.h > @@ -188,6 +188,7 @@ private: > details::StringSplitter split(const std::string &str, const std::string &delim); > > std::string libcameraBuildPath(); > +std::string libcameraSourcePath(); > > } /* namespace utils */ > > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > index 97f9b632e45b..056a67e93e0f 100644 > --- a/src/libcamera/utils.cpp > +++ b/src/libcamera/utils.cpp > @@ -360,9 +360,10 @@ bool isLibcameraInstalled() > * > * During development, it is useful to run libcamera binaries directly from the > * build directory without installing them. This function helps components that > - * need to locate resources, such as IPA modules or IPA proxy workers, by > - * providing them with the path to the root of the build directory. Callers can > - * then use it to complement or override searches in system-wide directories. > + * need to locate resources in the build tree, such as IPA modules or IPA proxy > + * workers, by providing them with the path to the root of the build directory. > + * Callers can then use it to complement or override searches in system-wide > + * directories. > * > * If libcamera has been installed, the build directory path is not available > * and this function returns an empty string. > @@ -385,6 +386,36 @@ std::string libcameraBuildPath() > return dirname(info.dli_fname) + "/../../"; > } > > +namespace { > +/* \todo Figure out a way to zero this section at install time */ > +__attribute__((section(".zero"))) > +const char sourceDirectory[] = LIBCAMERA_SOURCE_DIR; Even if we can zero this out - it won't give us reproducible builds, as the section size will be dependant upon the size of the LIBCAMERA_SOURCE_DIR string. Now I think about it - I can't recall if we hit the same issue on the buildPath too. If this 'works for now' we can defer though. > +} /* namespace */ > + > +/** > + * \brief Retrieve the path to the source directory > + * > + * During development, it is useful to run libcamera binaries directly from the > + * build directory without installing them. This function helps components that > + * need to locate resources in the source tree, such as IPA configuration > + * files, by providing them with the path to the root of the source directory. > + * Callers can then use it to complement or override searches in system-wide > + * directories. > + * > + * If libcamera has been installed, the source directory path is not available > + * and this function returns an empty string. > + * > + * \return The path to the source directory if running from a build, or an empty > + * string otherwise > + */ > +std::string libcameraSourcePath() > +{ > + if (isLibcameraInstalled()) > + return std::string(); > + > + return std::string(sourceDirectory) + "/"; > +} > + > } /* namespace utils */ > > } /* namespace libcamera */ >
Hi Kieran, On Mon, Apr 27, 2020 at 11:11:35AM +0100, Kieran Bingham wrote: > On 27/04/2020 04:17, Laurent Pinchart wrote: > > Similarly to libcameraBuildPath(), there's a need to locate resources > > within the source tree when running libcamera without installing it. > > Support this use case with a new utils::libcameraSourcePath() function. > > Eeeep. I know :-S > > The implementation currently leaks the path to the source tree in the > > installed binary. The data is put in a separate section named .zero > > which will need to be zeroed at installation time to fix this. > > Indeed. > > I guess we can't get any of this from the debug symbols can we? That > won't be reliable. Do we really want a debug symbols parser inside libcamera ? :-) > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > This is always going to be painful... but I don't think we should block > this branch just because of leaking the source path - but we certainly > want to fix it soon after. > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> I'll try another approach, based on a symlink to the source tree inside the build directory. If I can't do so in a reasonable amount of time I'll merge this patch as-is and fix the issue on top. > > --- > > meson.build | 2 ++ > > src/libcamera/include/utils.h | 1 + > > src/libcamera/utils.cpp | 37 ++++++++++++++++++++++++++++++++--- > > 3 files changed, 37 insertions(+), 3 deletions(-) > > > > diff --git a/meson.build b/meson.build > > index c6e6a934e54e..2058a6a77ceb 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -26,6 +26,8 @@ libcamera_version = libcamera_git_version.split('+')[0] > > cc = meson.get_compiler('c') > > config_h = configuration_data() > > > > +config_h.set('LIBCAMERA_SOURCE_DIR', '"' + meson.current_source_dir() + '"') > > + > > if cc.has_header_symbol('execinfo.h', 'backtrace') > > config_h.set('HAVE_BACKTRACE', 1) > > endif > > diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h > > index 242eeded9d50..3334ff16613d 100644 > > --- a/src/libcamera/include/utils.h > > +++ b/src/libcamera/include/utils.h > > @@ -188,6 +188,7 @@ private: > > details::StringSplitter split(const std::string &str, const std::string &delim); > > > > std::string libcameraBuildPath(); > > +std::string libcameraSourcePath(); > > > > } /* namespace utils */ > > > > diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp > > index 97f9b632e45b..056a67e93e0f 100644 > > --- a/src/libcamera/utils.cpp > > +++ b/src/libcamera/utils.cpp > > @@ -360,9 +360,10 @@ bool isLibcameraInstalled() > > * > > * During development, it is useful to run libcamera binaries directly from the > > * build directory without installing them. This function helps components that > > - * need to locate resources, such as IPA modules or IPA proxy workers, by > > - * providing them with the path to the root of the build directory. Callers can > > - * then use it to complement or override searches in system-wide directories. > > + * need to locate resources in the build tree, such as IPA modules or IPA proxy > > + * workers, by providing them with the path to the root of the build directory. > > + * Callers can then use it to complement or override searches in system-wide > > + * directories. > > * > > * If libcamera has been installed, the build directory path is not available > > * and this function returns an empty string. > > @@ -385,6 +386,36 @@ std::string libcameraBuildPath() > > return dirname(info.dli_fname) + "/../../"; > > } > > > > +namespace { > > +/* \todo Figure out a way to zero this section at install time */ > > +__attribute__((section(".zero"))) > > +const char sourceDirectory[] = LIBCAMERA_SOURCE_DIR; > > Even if we can zero this out - it won't give us reproducible builds, as > the section size will be dependant upon the size of the > LIBCAMERA_SOURCE_DIR string. > > Now I think about it - I can't recall if we hit the same issue on the > buildPath too. > > If this 'works for now' we can defer though. > > > +} /* namespace */ > > + > > +/** > > + * \brief Retrieve the path to the source directory > > + * > > + * During development, it is useful to run libcamera binaries directly from the > > + * build directory without installing them. This function helps components that > > + * need to locate resources in the source tree, such as IPA configuration > > + * files, by providing them with the path to the root of the source directory. > > + * Callers can then use it to complement or override searches in system-wide > > + * directories. > > + * > > + * If libcamera has been installed, the source directory path is not available > > + * and this function returns an empty string. > > + * > > + * \return The path to the source directory if running from a build, or an empty > > + * string otherwise > > + */ > > +std::string libcameraSourcePath() > > +{ > > + if (isLibcameraInstalled()) > > + return std::string(); > > + > > + return std::string(sourceDirectory) + "/"; > > +} > > + > > } /* namespace utils */ > > > > } /* namespace libcamera */
diff --git a/meson.build b/meson.build index c6e6a934e54e..2058a6a77ceb 100644 --- a/meson.build +++ b/meson.build @@ -26,6 +26,8 @@ libcamera_version = libcamera_git_version.split('+')[0] cc = meson.get_compiler('c') config_h = configuration_data() +config_h.set('LIBCAMERA_SOURCE_DIR', '"' + meson.current_source_dir() + '"') + if cc.has_header_symbol('execinfo.h', 'backtrace') config_h.set('HAVE_BACKTRACE', 1) endif diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h index 242eeded9d50..3334ff16613d 100644 --- a/src/libcamera/include/utils.h +++ b/src/libcamera/include/utils.h @@ -188,6 +188,7 @@ private: details::StringSplitter split(const std::string &str, const std::string &delim); std::string libcameraBuildPath(); +std::string libcameraSourcePath(); } /* namespace utils */ diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp index 97f9b632e45b..056a67e93e0f 100644 --- a/src/libcamera/utils.cpp +++ b/src/libcamera/utils.cpp @@ -360,9 +360,10 @@ bool isLibcameraInstalled() * * During development, it is useful to run libcamera binaries directly from the * build directory without installing them. This function helps components that - * need to locate resources, such as IPA modules or IPA proxy workers, by - * providing them with the path to the root of the build directory. Callers can - * then use it to complement or override searches in system-wide directories. + * need to locate resources in the build tree, such as IPA modules or IPA proxy + * workers, by providing them with the path to the root of the build directory. + * Callers can then use it to complement or override searches in system-wide + * directories. * * If libcamera has been installed, the build directory path is not available * and this function returns an empty string. @@ -385,6 +386,36 @@ std::string libcameraBuildPath() return dirname(info.dli_fname) + "/../../"; } +namespace { +/* \todo Figure out a way to zero this section at install time */ +__attribute__((section(".zero"))) +const char sourceDirectory[] = LIBCAMERA_SOURCE_DIR; +} /* namespace */ + +/** + * \brief Retrieve the path to the source directory + * + * During development, it is useful to run libcamera binaries directly from the + * build directory without installing them. This function helps components that + * need to locate resources in the source tree, such as IPA configuration + * files, by providing them with the path to the root of the source directory. + * Callers can then use it to complement or override searches in system-wide + * directories. + * + * If libcamera has been installed, the source directory path is not available + * and this function returns an empty string. + * + * \return The path to the source directory if running from a build, or an empty + * string otherwise + */ +std::string libcameraSourcePath() +{ + if (isLibcameraInstalled()) + return std::string(); + + return std::string(sourceDirectory) + "/"; +} + } /* namespace utils */ } /* namespace libcamera */
Similarly to libcameraBuildPath(), there's a need to locate resources within the source tree when running libcamera without installing it. Support this use case with a new utils::libcameraSourcePath() function. The implementation currently leaks the path to the source tree in the installed binary. The data is put in a separate section named .zero which will need to be zeroed at installation time to fix this. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- meson.build | 2 ++ src/libcamera/include/utils.h | 1 + src/libcamera/utils.cpp | 37 ++++++++++++++++++++++++++++++++--- 3 files changed, 37 insertions(+), 3 deletions(-)