| Message ID | 20230914092756.31516-1-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 90e0fea6c6ba6294bf9438d37d74676cc53ef924 |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Thu, Sep 14, 2023 at 10:27:56AM +0100, Naushir Patuck via libcamera-devel wrote: > If the json file parsing failed due to a malformed file, the root > pointer would be null. This was not tested and caused a segfault when > trying to use the pointer to retrive the version key. > > Fix this by bailing out early if the parser returns a null pointer. Does the parser complains about this, or should you add a message (or do you think it's not necessary ?) > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/controller.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > index fa1721130783..14d245da2ce7 100644 > --- a/src/ipa/rpi/controller/controller.cpp > +++ b/src/ipa/rpi/controller/controller.cpp > @@ -56,6 +56,9 @@ int Controller::read(char const *filename) > } > > std::unique_ptr<YamlObject> root = YamlParser::parse(file); > + if (!root) > + return -EINVAL; > + > double version = (*root)["version"].get<double>(1.0); > target_ = (*root)["target"].get<std::string>("bcm2835"); > > -- > 2.34.1 >
Hi Naush Thanks for the patch. On Thu, 14 Sept 2023 at 10:28, Naushir Patuck via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > If the json file parsing failed due to a malformed file, the root > pointer would be null. This was not tested and caused a segfault when > trying to use the pointer to retrive the version key. s/retrive/retrieve/ But otherwise: Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks David > > Fix this by bailing out early if the parser returns a null pointer. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/controller.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > index fa1721130783..14d245da2ce7 100644 > --- a/src/ipa/rpi/controller/controller.cpp > +++ b/src/ipa/rpi/controller/controller.cpp > @@ -56,6 +56,9 @@ int Controller::read(char const *filename) > } > > std::unique_ptr<YamlObject> root = YamlParser::parse(file); > + if (!root) > + return -EINVAL; > + > double version = (*root)["version"].get<double>(1.0); > target_ = (*root)["target"].get<std::string>("bcm2835"); > > -- > 2.34.1 >
Hi Jacopo, On Thu, 14 Sept 2023 at 10:43, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Naush > > On Thu, Sep 14, 2023 at 10:27:56AM +0100, Naushir Patuck via libcamera-devel wrote: > > If the json file parsing failed due to a malformed file, the root > > pointer would be null. This was not tested and caused a segfault when > > trying to use the pointer to retrive the version key. > > > > Fix this by bailing out early if the parser returns a null pointer. > > Does the parser complains about this, or should you add a message (or > do you think it's not necessary ?) Yes, the parser provides the error message, so I did not think it was needed here as well: ERROR YamlParser yaml_parser.cpp:844 Failed to parse YAML content from /usr/local/share/libcamera/ipa/rpi/vc4/imx219.json Regards, Naush > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > src/ipa/rpi/controller/controller.cpp | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > > index fa1721130783..14d245da2ce7 100644 > > --- a/src/ipa/rpi/controller/controller.cpp > > +++ b/src/ipa/rpi/controller/controller.cpp > > @@ -56,6 +56,9 @@ int Controller::read(char const *filename) > > } > > > > std::unique_ptr<YamlObject> root = YamlParser::parse(file); > > + if (!root) > > + return -EINVAL; > > + > > double version = (*root)["version"].get<double>(1.0); > > target_ = (*root)["target"].get<std::string>("bcm2835"); > > > > -- > > 2.34.1 > >
On Thu, Sep 14, 2023 at 10:44:59AM +0100, Naushir Patuck via libcamera-devel wrote: > Hi Jacopo, > > On Thu, 14 Sept 2023 at 10:43, Jacopo Mondi > <jacopo.mondi@ideasonboard.com> wrote: > > > > Hi Naush > > > > On Thu, Sep 14, 2023 at 10:27:56AM +0100, Naushir Patuck via libcamera-devel wrote: > > > If the json file parsing failed due to a malformed file, the root > > > pointer would be null. This was not tested and caused a segfault when > > > trying to use the pointer to retrive the version key. > > > > > > Fix this by bailing out early if the parser returns a null pointer. > > > > Does the parser complains about this, or should you add a message (or > > do you think it's not necessary ?) > > Yes, the parser provides the error message, so I did not think it was > needed here as well: > > ERROR YamlParser yaml_parser.cpp:844 Failed to parse YAML content from > /usr/local/share/libcamera/ipa/rpi/vc4/imx219.json Great then! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Regards, > Naush > > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > src/ipa/rpi/controller/controller.cpp | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp > > > index fa1721130783..14d245da2ce7 100644 > > > --- a/src/ipa/rpi/controller/controller.cpp > > > +++ b/src/ipa/rpi/controller/controller.cpp > > > @@ -56,6 +56,9 @@ int Controller::read(char const *filename) > > > } > > > > > > std::unique_ptr<YamlObject> root = YamlParser::parse(file); > > > + if (!root) > > > + return -EINVAL; > > > + > > > double version = (*root)["version"].get<double>(1.0); > > > target_ = (*root)["target"].get<std::string>("bcm2835"); > > > > > > -- > > > 2.34.1 > > >
diff --git a/src/ipa/rpi/controller/controller.cpp b/src/ipa/rpi/controller/controller.cpp index fa1721130783..14d245da2ce7 100644 --- a/src/ipa/rpi/controller/controller.cpp +++ b/src/ipa/rpi/controller/controller.cpp @@ -56,6 +56,9 @@ int Controller::read(char const *filename) } std::unique_ptr<YamlObject> root = YamlParser::parse(file); + if (!root) + return -EINVAL; + double version = (*root)["version"].get<double>(1.0); target_ = (*root)["target"].get<std::string>("bcm2835");
If the json file parsing failed due to a malformed file, the root pointer would be null. This was not tested and caused a segfault when trying to use the pointer to retrive the version key. Fix this by bailing out early if the parser returns a null pointer. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/ipa/rpi/controller/controller.cpp | 3 +++ 1 file changed, 3 insertions(+)