[PATCH] libcamera: yaml_parser: Output more details when parsing fails

[Stefan Klug]

Hi Laurent,

Thank you for the review. 

On Thu, Dec 05, 2024 at 10:12:43PM +0200, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Wed, Dec 04, 2024 at 12:10:13PM +0100, Stefan Klug wrote:
> > On malformed yaml files, the yaml parser only errors out without giving
> > details on the error that happened. Fix that by providing a more detailed
> > error message.
> > 
> > Output old:
> > 
> > ERROR YamlParser yaml_parser.cpp:886 Failed to parse YAML content from /root/imx283.yaml
> > 
> > Output new:
> > 
> > ERROR YamlParser yaml_parser.cpp:627 /root/imx283.yaml:72:8 could not find expected ':' while scanning a simple key
> > ERROR YamlParser yaml_parser.cpp:886 Failed to parse YAML content from /root/imx283.yaml
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/libcamera/yaml_parser.cpp | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> > index db256ec5b04d..f9302c4ee3fa 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -509,8 +509,17 @@ YamlParserContext::EventPtr YamlParserContext::nextEvent()
> 
> I wonder what else you have in your tree if the message prints line 627,
> as indicated in the example in the commit message :-)

Interesting indeed. I checked the logs. The patch was written just
before you merged the "Use std::from_chars()" cleanup that removed quite
some code. I guess that explains the difference.

Regards,
Stefan

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> >  	EventPtr event(new yaml_event_t);
> >  
> >  	/* yaml_parser_parse returns 1 when it succeeds */
> > -	if (!yaml_parser_parse(&parser_, event.get()))
> > +	if (!yaml_parser_parse(&parser_, event.get())) {
> > +		File *file = static_cast<File *>(parser_.read_handler_data);
> > +
> > +		LOG(YamlParser, Error) << file->fileName() << ":"
> > +				       << parser_.problem_mark.line << ":"
> > +				       << parser_.problem_mark.column << " "
> > +				       << parser_.problem << " "
> > +				       << parser_.context;
> > +
> >  		return nullptr;
> > +	}
> >  
> >  	return event;
> >  }
> 
> -- 
> Regards,
> 
> Laurent Pinchart