GNOME Bugzilla – Bug 708736
Add support for using GBytes with GcrParser
Last modified: 2019-02-22 11:59:36 UTC
When we migrated to using GBytes in the Gcr library, we didn't add support for using GBytes with GcrParser. A TODO is present. This is a problem because GcrParser and GcrParsed assume that they can hold a reference to the input data.
Created attachment 255675 [details] [review] gcr: Allow using GBytes with GcrParser This also solves a corner case where GcrParser would keep around a pointer to the data passed into gcr_parser_parse_data(), even after that data had been freed. gcr_parser_parse_data() now copies the data passed in, where as the new gcr_parser_parse_bytes() simply keeps a reference to the GBytes.
Review of attachment 255675 [details] [review]: Looks mostly good. How can I run the tests though? ::: gcr/gcr-parser.c @@ +2395,3 @@ + if (g_bytes_get_size (data) > 0) { + args.data = g_bytes_ref (data); Should there be a corresponding g_bytes_unref in the finalize function? ::: gcr/tests/test-parser.c @@ +266,3 @@ + fis = g_file_read (file, NULL, &error); + g_assert_no_error (error); + Do you want to connect to the "parsed" signal here like in test_parsed_bytes?
(In reply to comment #3) > Review of attachment 255675 [details] [review]: > > Looks mostly good. How can I run the tests though? $ make check > ::: gcr/gcr-parser.c > @@ +2395,3 @@ > > + if (g_bytes_get_size (data) > 0) { > + args.data = g_bytes_ref (data); > > Should there be a corresponding g_bytes_unref in the finalize function? It's down below at the end of the statement block. > ::: gcr/tests/test-parser.c > @@ +266,3 @@ > + fis = g_file_read (file, NULL, &error); > + g_assert_no_error (error); > + > > Do you want to connect to the "parsed" signal here like in test_parsed_bytes? Because we're not checking that anything is parsed. I guess I should connect to the "parsed" signal to make sure it gets called.
Created attachment 256096 [details] [review] gcr: Allow using GBytes with GcrParser This also solves a corner case where GcrParser would keep around a pointer to the data passed into gcr_parser_parse_data(), even after that data had been freed. gcr_parser_parse_data() now copies the data passed in, where as the new gcr_parser_parse_bytes() simply keeps a reference to the GBytes.
Review of attachment 256096 [details] [review]: Looks good to merge in.
Attachment 256096 [details] pushed as 6924fa9 - gcr: Allow using GBytes with GcrParser