[librsvg] (#581): Don't call g_warning and g_critical from the C code



commit dad5ce86ef2429d35e1666fac4eb0d64443cfc54
Author: Federico Mena Quintero <federico gnome org>
Date:   Wed Apr 8 14:43:34 2020 -0500

    (#581): Don't call g_warning and g_critical from the C code
    
    g_warning() and g_critical() are C macros, not available in Rust
    code.  To call them, we were defining wrapper functions in the C code
    and up-calling to them from Rust.
    
    This is problematic with the linker in Debian powerpc, and is the one
    remaining wart in having to call from Rust to C shims.
    
    This commit effectively implements what g_warning()/g_critical() do
    but directly from Rust.  Glib's C code would call
    g_log_structured_standard(), which would make an argument list for
    g_log_structured_array().
    
    Since we don't have all the information available that
    g_log_structured_standard() needs (we are missing the file and line
    number, but those are not very useful in these particular log messages
    anyway), we'll call g_log_structured_array() by hand.
    
    Fixes https://gitlab.gnome.org/GNOME/librsvg/-/issues/581

 NEWS                      |  4 +++
 configure.ac              |  2 +-
 librsvg/Cargo.toml        |  4 +--
 librsvg/c_api.rs          | 84 +++++++++++++++++++++++++++++++++++++++--------
 librsvg/rsvg-base.c       | 21 ------------
 librsvg_crate/Cargo.toml  |  2 +-
 rsvg_internals/Cargo.toml |  2 +-
 7 files changed, 80 insertions(+), 39 deletions(-)
---
diff --git a/NEWS b/NEWS
index b7154299..5fba43a6 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Version 2.49.0
+
+- Librsvg now requires glib 2.50.0 or later.
+
 Version 2.47.3
 
 - #379 - New API, rsvg_handle_set_stylesheet(), to set a CSS
diff --git a/configure.ac b/configure.ac
index a4ff93ce..9f88860f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -73,7 +73,7 @@ dnl This corresponds to Freetype2 2.8
 FREETYPE2_REQUIRED=20.0.14
 GDK_PIXBUF_REQUIRED=2.20
 GIO_REQUIRED=2.24.0
-GLIB_REQUIRED=2.48.0
+GLIB_REQUIRED=2.50.0
 HARFBUZZ_REQUIRED=2.0.0
 LIBXML_REQUIRED=2.9.0
 PANGO_REQUIRED=1.38.0
diff --git a/librsvg/Cargo.toml b/librsvg/Cargo.toml
index 1f41d660..0fed1a98 100644
--- a/librsvg/Cargo.toml
+++ b/librsvg/Cargo.toml
@@ -17,8 +17,8 @@ cairo-sys-rs = "0.9.0"
 gdk-pixbuf = "0.8.0"
 gdk-pixbuf-sys = "0.9.0"
 glib = "0.9.0"
-glib-sys = "0.9.0"
-gio = { version="0.8.0", features=["v2_48"] } # per configure.ac
+glib-sys = { version="0.9.0", features=["v2_50"] }
+gio = { version="0.8.0", features=["v2_50"] } # per configure.ac
 gio-sys = "0.9.0"
 gobject-sys = "0.9.0"
 libc = "0.2"
diff --git a/librsvg/c_api.rs b/librsvg/c_api.rs
index 9ffa966b..8bdb59a1 100644
--- a/librsvg/c_api.rs
+++ b/librsvg/c_api.rs
@@ -27,6 +27,8 @@ use glib::{
     ToValue, Type, Value,
 };
 
+use glib_sys::{G_LOG_LEVEL_WARNING, G_LOG_LEVEL_CRITICAL, GLogField, g_log_structured_array};
+
 use gobject_sys::{GEnumValue, GFlagsValue};
 
 use rsvg_internals::{
@@ -1555,24 +1557,80 @@ fn warn_on_invalid_id(e: RenderingError) -> RenderingError {
     e
 }
 
-fn rsvg_g_warning(msg: &str) {
-    unsafe {
-        extern "C" {
-            fn rsvg_g_warning_from_c(msg: *const libc::c_char);
-        }
+/*
+  G_LOG_LEVEL_CRITICAL          = 1 << 3,
+  G_LOG_LEVEL_WARNING           = 1 << 4,
+
+#define g_critical(...) g_log_structured_standard (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, \
+                                                   __FILE__, G_STRINGIFY (__LINE__), \
+                                                   G_STRFUNC, __VA_ARGS__)
+#define g_warning(...)  g_log_structured_standard (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, \
+                                                   __FILE__, G_STRINGIFY (__LINE__), \
+                                                   G_STRFUNC, __VA_ARGS__)
+  GLogField fields[] =
+    {
+      { "PRIORITY", log_level_to_priority (log_level), -1 },
+      { "CODE_FILE", file, -1 },
+      { "CODE_LINE", line, -1 },
+      { "CODE_FUNC", func, -1 },
+      /* Filled in later: */
+      { "MESSAGE", NULL, -1 },
+      /* If @log_domain is %NULL, we will not pass this field: */
+      { "GLIB_DOMAIN", log_domain, -1 },
+    };
+
+  g_log_structured_array (log_level, fields, n_fields);
+ */
+
+/// Helper for `rsvg_g_warning` and `rsvg_g_critical`
+///
+/// This simulates what in C would be a call to the g_warning() or g_critical()
+/// macros, but with the underlying function g_log_structured_array().
+///
+/// If the implementation of g_warning() or g_critical() changes, we'll have
+/// to change this function.
+fn rsvg_g_log(level: glib_sys::GLogLevelFlags, msg: &str) {
+    // stolen from gmessages.c:log_level_to_priority()
+    let priority = match level {
+        G_LOG_LEVEL_WARNING | G_LOG_LEVEL_CRITICAL => b"4\0",
+        _ => unreachable!("please add another log level priority to rsvg_g_log()"),
+    };
 
-        rsvg_g_warning_from_c(msg.to_glib_none().0);
+    // Glib's g_log_structured_standard() adds a few more fields for the source
+    // file, line number, etc., but those are not terribly useful without a stack
+    // trace.  So, we'll omit them.
+    let fields = [
+        GLogField {
+            key: b"PRIORITY\0" as *const u8 as *const _,
+            value: priority as *const u8 as *const _,
+            length: -1,
+        },
+
+        GLogField {
+            key: b"MESSAGE\0" as *const u8 as *const _,
+            value: msg.as_ptr() as *const _,
+            length: msg.len() as _,
+        },
+
+        // This is the G_LOG_DOMAIN set from the Makefile
+        GLogField {
+            key: b"GLIB_DOMAIN\0" as *const u8 as *const _,
+            value: b"librsvg\0" as *const u8 as *const _,
+            length: -1,
+        },
+    ];
+
+    unsafe {
+        g_log_structured_array(level, fields.as_ptr(), fields.len());
     }
 }
 
-fn rsvg_g_critical(msg: &str) {
-    unsafe {
-        extern "C" {
-            fn rsvg_g_critical_from_c(msg: *const libc::c_char);
-        }
+fn rsvg_g_warning(msg: &str) {
+    rsvg_g_log(glib_sys::G_LOG_LEVEL_WARNING, msg);
+}
 
-        rsvg_g_critical_from_c(msg.to_glib_none().0);
-    }
+fn rsvg_g_critical(msg: &str) {
+    rsvg_g_log(glib_sys::G_LOG_LEVEL_CRITICAL, msg);
 }
 
 pub(crate) fn set_gerror(err: *mut *mut glib_sys::GError, code: u32, msg: &str) {
diff --git a/librsvg/rsvg-base.c b/librsvg/rsvg-base.c
index 71ba5b37..1f7d9208 100644
--- a/librsvg/rsvg-base.c
+++ b/librsvg/rsvg-base.c
@@ -134,24 +134,3 @@ rsvg_css_parse_color_ (const char *str)
 {
     return rsvg_css_parse_color (str);
 }
-
-/* These functions exist just so that we can effectively call g_warning() or
- * g_critical() from Rust, since glib-rs doesn't bind the g_log functions yet.
- */
-G_GNUC_INTERNAL
-void rsvg_g_warning_from_c(const char *msg);
-
-G_GNUC_INTERNAL
-void rsvg_g_critical_from_c(const char *msg);
-
-void
-rsvg_g_warning_from_c(const char *msg)
-{
-    g_warning ("%s", msg);
-}
-
-void
-rsvg_g_critical_from_c(const char *msg)
-{
-    g_critical ("%s", msg);
-}
diff --git a/librsvg_crate/Cargo.toml b/librsvg_crate/Cargo.toml
index 5c3bbbb4..3d167fbb 100644
--- a/librsvg_crate/Cargo.toml
+++ b/librsvg_crate/Cargo.toml
@@ -11,7 +11,7 @@ name = "librsvg"
 [dependencies]
 cairo-rs = "0.8.0"
 glib = "0.9.0"
-gio = { version="0.8.0", features=["v2_48"] } # per configure.ac
+gio = { version="0.8.0", features=["v2_50"] } # per configure.ac
 rsvg_internals = { path = "../rsvg_internals" }
 url = "2"
 
diff --git a/rsvg_internals/Cargo.toml b/rsvg_internals/Cargo.toml
index ad6fe4da..b7dee729 100644
--- a/rsvg_internals/Cargo.toml
+++ b/rsvg_internals/Cargo.toml
@@ -16,7 +16,7 @@ encoding = "0.2.33"
 float-cmp = "0.6.0"
 gdk-pixbuf = "0.8.0"
 gdk-pixbuf-sys = "0.9.0"
-gio = { version="0.8.0", features=["v2_48"] } # per configure.ac
+gio = { version="0.8.0", features=["v2_50"] } # per configure.ac
 gio-sys = "0.9.0"
 glib = "0.9.0"
 glib-sys = "0.9.0"


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]