| Message ID | 20220826113931.59323-2-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Tomi, Thank you for the patch. On Fri, Aug 26, 2022 at 02:39:30PM +0300, Tomi Valkeinen via libcamera-devel wrote: > LogCategory just stores the char * that was given to it in the > constructor, i.e. it refers to memory "outside" LogCategory. If the > LogCategory is defined in a .so that is unloaded, then it leads to the > LogCategory pointing to freed memory, causing a crash. > > Fix this by taking a copy of the name by using a std::string instead of > just storing the pointer. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I won't apply this yet though, as it doesn't help much without 2/2, in case this patch needs to change in v2 due to changes in 2/2. > --- > include/libcamera/base/log.h | 4 ++-- > src/libcamera/base/log.cpp | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h > index 3fc5ced3..8b462767 100644 > --- a/include/libcamera/base/log.h > +++ b/include/libcamera/base/log.h > @@ -31,14 +31,14 @@ class LogCategory > public: > explicit LogCategory(const char *name); > > - const char *name() const { return name_; } > + const std::string &name() const { return name_; } > LogSeverity severity() const { return severity_; } > void setSeverity(LogSeverity severity); > > static const LogCategory &defaultCategory(); > > private: > - const char *name_; > + const std::string name_; > LogSeverity severity_; > }; > > diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp > index 5c359a22..a4a5b452 100644 > --- a/src/libcamera/base/log.cpp > +++ b/src/libcamera/base/log.cpp > @@ -568,7 +568,7 @@ void Logger::logSetLevel(const char *category, const char *level) > return; > > for (LogCategory *c : categories_) { > - if (!strcmp(c->name(), category)) { > + if (c->name() == category) { > c->setSeverity(severity); > break; > }
On 27/08/2022 04:28, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Fri, Aug 26, 2022 at 02:39:30PM +0300, Tomi Valkeinen via libcamera-devel wrote: >> LogCategory just stores the char * that was given to it in the >> constructor, i.e. it refers to memory "outside" LogCategory. If the >> LogCategory is defined in a .so that is unloaded, then it leads to the >> LogCategory pointing to freed memory, causing a crash. >> >> Fix this by taking a copy of the name by using a std::string instead of >> just storing the pointer. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I won't apply this yet though, as it doesn't help much without 2/2, in > case this patch needs to change in v2 due to changes in 2/2. This does help alone, as it fixes a crash. The issues the second patch fixes are not fatal. But no hurry to apply this, I think, as "no one" constructs CameraManagers multiple times... So lets get the second patch ready before applying this. Tomi
diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 3fc5ced3..8b462767 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -31,14 +31,14 @@ class LogCategory public: explicit LogCategory(const char *name); - const char *name() const { return name_; } + const std::string &name() const { return name_; } LogSeverity severity() const { return severity_; } void setSeverity(LogSeverity severity); static const LogCategory &defaultCategory(); private: - const char *name_; + const std::string name_; LogSeverity severity_; }; diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 5c359a22..a4a5b452 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -568,7 +568,7 @@ void Logger::logSetLevel(const char *category, const char *level) return; for (LogCategory *c : categories_) { - if (!strcmp(c->name(), category)) { + if (c->name() == category) { c->setSeverity(severity); break; }
LogCategory just stores the char * that was given to it in the constructor, i.e. it refers to memory "outside" LogCategory. If the LogCategory is defined in a .so that is unloaded, then it leads to the LogCategory pointing to freed memory, causing a crash. Fix this by taking a copy of the name by using a std::string instead of just storing the pointer. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- include/libcamera/base/log.h | 4 ++-- src/libcamera/base/log.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)